From 63c2013ec1e52eb57eb09add60dba739a274ea4e Mon Sep 17 00:00:00 2001 From: hc-github-team-nomad-core <82989552+hc-github-team-nomad-core@users.noreply.github.com> Date: Fri, 20 Oct 2023 02:35:54 -0500 Subject: [PATCH] backport of commit ca9e08e6b5eee00d055b9429df5976a70cdcb2d6 (#18813) Co-authored-by: James Rasell --- .changelog/18795.txt | 12 +++ client/agent_endpoint.go | 5 +- client/structs/structs.go | 5 ++ command/agent/agent_endpoint.go | 22 +++-- command/agent/agent_endpoint_test.go | 13 +++ command/agent_monitor.go | 37 ++++---- command/agent_monitor_test.go | 9 ++ command/operator_debug.go | 84 ++++++++++--------- nomad/client_agent_endpoint.go | 5 +- website/content/api-docs/agent.mdx | 7 +- website/content/docs/commands/monitor.mdx | 3 + .../content/docs/commands/operator/debug.mdx | 3 + 12 files changed, 141 insertions(+), 64 deletions(-) create mode 100644 .changelog/18795.txt diff --git a/.changelog/18795.txt b/.changelog/18795.txt new file mode 100644 index 000000000..da2902baf --- /dev/null +++ b/.changelog/18795.txt @@ -0,0 +1,12 @@ +```release-note:improvement +cli: Added `log-include-location` flag to the `monitor` command +``` + +```release-note:improvement +cli: Added `log-include-location` flag to the `operator debug` command +``` + +```release-note:improvement +api: Added support for the `log_include_location` query parameter within the +`/v1/agent/monitor` HTTP endpoint +``` \ No newline at end of file diff --git a/client/agent_endpoint.go b/client/agent_endpoint.go index e0d4636f3..74bc31439 100644 --- a/client/agent_endpoint.go +++ b/client/agent_endpoint.go @@ -119,8 +119,9 @@ func (a *Agent) monitor(conn io.ReadWriteCloser) { defer cancel() monitor := monitor.New(512, a.c.logger, &log.LoggerOptions{ - JSONFormat: args.LogJSON, - Level: logLevel, + JSONFormat: args.LogJSON, + Level: logLevel, + IncludeLocation: args.LogIncludeLocation, }) frames := make(chan *sframer.StreamFrame, streamFramesBuffer) diff --git a/client/structs/structs.go b/client/structs/structs.go index 3e0340c32..08c22397b 100644 --- a/client/structs/structs.go +++ b/client/structs/structs.go @@ -45,6 +45,11 @@ type MonitorRequest struct { // LogJSON specifies if log format should be unstructured or json LogJSON bool + // LogIncludeLocation dictates whether the logger includes file and line + // information on each log line. This is useful for Nomad development and + // debugging. + LogIncludeLocation bool + // NodeID is the node we want to track the logs of NodeID string diff --git a/command/agent/agent_endpoint.go b/command/agent/agent_endpoint.go index afd7991c9..7b7e02322 100644 --- a/command/agent/agent_endpoint.go +++ b/command/agent/agent_endpoint.go @@ -179,14 +179,26 @@ func (s *HTTPServer) AgentMonitor(resp http.ResponseWriter, req *http.Request) ( plainText = parsed } + logIncludeLocation := false + logIncludeLocationStr := req.URL.Query().Get("log_include_location") + if logIncludeLocationStr != "" { + parsed, err := strconv.ParseBool(logIncludeLocationStr) + if err != nil { + return nil, CodedError(http.StatusBadRequest, + fmt.Sprintf("Unknown option for log_include_location: %v", err)) + } + logIncludeLocation = parsed + } + nodeID := req.URL.Query().Get("node_id") // Build the request and parse the ACL token args := cstructs.MonitorRequest{ - NodeID: nodeID, - ServerID: req.URL.Query().Get("server_id"), - LogLevel: logLevel, - LogJSON: logJSON, - PlainText: plainText, + NodeID: nodeID, + ServerID: req.URL.Query().Get("server_id"), + LogLevel: logLevel, + LogJSON: logJSON, + LogIncludeLocation: logIncludeLocation, + PlainText: plainText, } // if node and server were requested return error diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 1272924a2..ab1337830 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -31,6 +31,7 @@ import ( "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" + "github.com/shoenig/test/must" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -293,6 +294,18 @@ func TestHTTP_AgentMonitor(t *testing.T) { }) }) + t.Run("unknown log_include_location", func(t *testing.T) { + httpTest(t, nil, func(s *TestAgent) { + req, err := http.NewRequest(http.MethodGet, "/v1/agent/monitor?log_include_location=maybe", nil) + must.NoError(t, err) + resp := newClosableRecorder() + + // Make the request + _, err = s.Server.AgentMonitor(resp, req) + must.Eq(t, http.StatusBadRequest, err.(HTTPCodedError).Code()) + }) + }) + t.Run("check for specific log level", func(t *testing.T) { httpTest(t, nil, func(s *TestAgent) { req, err := http.NewRequest(http.MethodGet, "/v1/agent/monitor?log_level=warn", nil) diff --git a/command/agent_monitor.go b/command/agent_monitor.go index c1c6d8d52..07eac2631 100644 --- a/command/agent_monitor.go +++ b/command/agent_monitor.go @@ -19,6 +19,13 @@ import ( type MonitorCommand struct { Meta + + // Below this point is where CLI flag options are stored. + logLevel string + nodeID string + serverID string + logJSON bool + logIncludeLocation bool } func (c *MonitorCommand) Help() string { @@ -42,6 +49,9 @@ Monitor Specific Options: -log-level Sets the log level to monitor (default: INFO) + -log-include-location + Include file and line information in each log line. The default is false. + -node-id Sets the specific node to monitor @@ -68,17 +78,13 @@ func (c *MonitorCommand) Run(args []string) int { Ui: c.Ui, } - var logLevel string - var nodeID string - var serverID string - var logJSON bool - flags := c.Meta.FlagSet(c.Name(), FlagSetClient) flags.Usage = func() { c.Ui.Output(c.Help()) } - flags.StringVar(&logLevel, "log-level", "", "") - flags.StringVar(&nodeID, "node-id", "", "") - flags.StringVar(&serverID, "server-id", "", "") - flags.BoolVar(&logJSON, "json", false, "") + flags.StringVar(&c.logLevel, "log-level", "", "") + flags.BoolVar(&c.logIncludeLocation, "log-include-location", false, "") + flags.StringVar(&c.nodeID, "node-id", "", "") + flags.StringVar(&c.serverID, "server-id", "", "") + flags.BoolVar(&c.logJSON, "json", false, "") if err := flags.Parse(args); err != nil { return 1 @@ -99,8 +105,8 @@ func (c *MonitorCommand) Run(args []string) int { } // Query the node info and lookup prefix - if nodeID != "" { - nodeID, err = lookupNodeID(client.Nodes(), nodeID) + if c.nodeID != "" { + c.nodeID, err = lookupNodeID(client.Nodes(), c.nodeID) if err != nil { c.Ui.Error(err.Error()) return 1 @@ -108,10 +114,11 @@ func (c *MonitorCommand) Run(args []string) int { } params := map[string]string{ - "log_level": logLevel, - "node_id": nodeID, - "server_id": serverID, - "log_json": strconv.FormatBool(logJSON), + "log_level": c.logLevel, + "node_id": c.nodeID, + "server_id": c.serverID, + "log_json": strconv.FormatBool(c.logJSON), + "log_include_location": strconv.FormatBool(c.logIncludeLocation), } query := &api.QueryOptions{ diff --git a/command/agent_monitor_test.go b/command/agent_monitor_test.go index 6cc26e176..ca121dde8 100644 --- a/command/agent_monitor_test.go +++ b/command/agent_monitor_test.go @@ -42,4 +42,13 @@ func TestMonitorCommand_Fails(t *testing.T) { out = ui.ErrorWriter.String() must.StrContains(t, out, "No node(s) with prefix") + + ui.ErrorWriter.Reset() + + // Fails on passing a log-include-location flag which cannot be parsed. + code = cmd.Run([]string{"-address=" + url, "-log-include-location=maybe"}) + must.One(t, code) + + out = ui.ErrorWriter.String() + must.StrContains(t, out, `invalid boolean value "maybe" for -log-include-location`) } diff --git a/command/operator_debug.go b/command/operator_debug.go index 42057708d..91c1ace65 100644 --- a/command/operator_debug.go +++ b/command/operator_debug.go @@ -39,28 +39,29 @@ import ( type OperatorDebugCommand struct { Meta - timestamp string - collectDir string - duration time.Duration - interval time.Duration - pprofInterval time.Duration - pprofDuration time.Duration - logLevel string - maxNodes int - nodeClass string - nodeIDs []string - serverIDs []string - topics map[api.Topic][]string - index uint64 - consul *external - vault *external - manifest []string - ctx context.Context - cancel context.CancelFunc - opts *api.QueryOptions - verbose bool - members *api.ServerMembers - nodes []*api.NodeListStub + timestamp string + collectDir string + duration time.Duration + interval time.Duration + pprofInterval time.Duration + pprofDuration time.Duration + logLevel string + logIncludeLocation bool + maxNodes int + nodeClass string + nodeIDs []string + serverIDs []string + topics map[api.Topic][]string + index uint64 + consul *external + vault *external + manifest []string + ctx context.Context + cancel context.CancelFunc + opts *api.QueryOptions + verbose bool + members *api.ServerMembers + nodes []*api.NodeListStub } const ( @@ -178,6 +179,10 @@ Debug Options: -log-level= The log level to monitor. Defaults to DEBUG. + -log-include-location + Include file and line information in each log line monitored. The default + is true. + -max-nodes= Cap the maximum number of client nodes included in the capture. Defaults to 10, set to 0 for unlimited. @@ -225,20 +230,21 @@ func (c *OperatorDebugCommand) Synopsis() string { func (c *OperatorDebugCommand) AutocompleteFlags() complete.Flags { return mergeAutocompleteFlags(c.Meta.AutocompleteFlags(FlagSetClient), complete.Flags{ - "-duration": complete.PredictAnything, - "-event-index": complete.PredictAnything, - "-event-topic": complete.PredictAnything, - "-interval": complete.PredictAnything, - "-log-level": complete.PredictSet("TRACE", "DEBUG", "INFO", "WARN", "ERROR"), - "-max-nodes": complete.PredictAnything, - "-node-class": NodeClassPredictor(c.Client), - "-node-id": NodePredictor(c.Client), - "-server-id": ServerPredictor(c.Client), - "-output": complete.PredictDirs("*"), - "-pprof-duration": complete.PredictAnything, - "-consul-token": complete.PredictAnything, - "-vault-token": complete.PredictAnything, - "-verbose": complete.PredictAnything, + "-duration": complete.PredictAnything, + "-event-index": complete.PredictAnything, + "-event-topic": complete.PredictAnything, + "-interval": complete.PredictAnything, + "-log-level": complete.PredictSet("TRACE", "DEBUG", "INFO", "WARN", "ERROR"), + "-log-include-location": complete.PredictAnything, + "-max-nodes": complete.PredictAnything, + "-node-class": NodeClassPredictor(c.Client), + "-node-id": NodePredictor(c.Client), + "-server-id": ServerPredictor(c.Client), + "-output": complete.PredictDirs("*"), + "-pprof-duration": complete.PredictAnything, + "-consul-token": complete.PredictAnything, + "-vault-token": complete.PredictAnything, + "-verbose": complete.PredictAnything, }) } @@ -358,6 +364,7 @@ func (c *OperatorDebugCommand) Run(args []string) int { flags.StringVar(&eventTopic, "event-topic", "none", "") flags.StringVar(&interval, "interval", "30s", "") flags.StringVar(&c.logLevel, "log-level", "DEBUG", "") + flags.BoolVar(&c.logIncludeLocation, "log-include-location", true, "") flags.IntVar(&c.maxNodes, "max-nodes", 10, "") flags.StringVar(&c.nodeClass, "node-class", "", "") flags.StringVar(&nodeIDs, "node-id", "all", "") @@ -769,8 +776,9 @@ func (c *OperatorDebugCommand) startMonitor(path, idKey, nodeID string, client * qo := api.QueryOptions{ Params: map[string]string{ - idKey: nodeID, - "log_level": c.logLevel, + idKey: nodeID, + "log_level": c.logLevel, + "log_include_location": strconv.FormatBool(c.logIncludeLocation), }, AllowStale: c.queryOpts().AllowStale, } diff --git a/nomad/client_agent_endpoint.go b/nomad/client_agent_endpoint.go index a3cc070bf..b4e73befc 100644 --- a/nomad/client_agent_endpoint.go +++ b/nomad/client_agent_endpoint.go @@ -198,8 +198,9 @@ func (a *Agent) monitor(conn io.ReadWriteCloser) { defer cancel() monitor := monitor.New(512, a.srv.logger, &log.LoggerOptions{ - Level: logLevel, - JSONFormat: args.LogJSON, + Level: logLevel, + JSONFormat: args.LogJSON, + IncludeLocation: args.LogIncludeLocation, }) frames := make(chan *sframer.StreamFrame, 32) diff --git a/website/content/api-docs/agent.mdx b/website/content/api-docs/agent.mdx index 57dca70fa..0331407b2 100644 --- a/website/content/api-docs/agent.mdx +++ b/website/content/api-docs/agent.mdx @@ -441,8 +441,8 @@ The table below shows this endpoint's support for ### Parameters - `node` `(string: )` - Specifies the name of the node to force leave. -- `prune` `(boolean: )` - Removes failed or left server from the Serf - member list immediately. If member is actually still alive, it will eventually rejoin +- `prune` `(boolean: )` - Removes failed or left server from the Serf + member list immediately. If member is actually still alive, it will eventually rejoin the cluster again. @@ -605,6 +605,9 @@ The table below shows this endpoint's support for - `log_json` `(bool: false)` - Specifies if the log format for streamed logs should be JSON. +- `log_include_location` `(bool: false)` - Specifies if the logs streamed should + include file and line information. + - `node_id` `(string: "a57b2adb-1a30-2dda-8df0-25abb0881952")` - Specifies a text string containing a node-id to target for streaming. diff --git a/website/content/docs/commands/monitor.mdx b/website/content/docs/commands/monitor.mdx index ff0738014..445dab500 100644 --- a/website/content/docs/commands/monitor.mdx +++ b/website/content/docs/commands/monitor.mdx @@ -36,6 +36,9 @@ capability. - `-log-level`: The log level to use for log streaming. Defaults to `info`. Possible values include `trace`, `debug`, `info`, `warn`, `error` +- `-log-include-location`: Include file and line information in each log line. + The default is `false`. + - `-node-id`: Specifies the client node-id to stream logs from. If no node-id is given the nomad server from the -address flag will be used. diff --git a/website/content/docs/commands/operator/debug.mdx b/website/content/docs/commands/operator/debug.mdx index a36e35b53..362bdfe10 100644 --- a/website/content/docs/commands/operator/debug.mdx +++ b/website/content/docs/commands/operator/debug.mdx @@ -55,6 +55,9 @@ true. - `-log-level=DEBUG`: The log level to monitor. Defaults to `DEBUG`. +- `-log-include-location`: Include file and line information in each log line + monitored. The default is `true`. + - `-max-nodes=`: Cap the maximum number of client nodes included in the capture. Defaults to 10, set to 0 for unlimited.