Nomad Monitor #6499

drewbailey · 2019-10-15T20:48:05Z

This PR adds a new top level command nomad monitor.

This command is similar to consul monitor in that it is used to stream logs using a configurable log level without needing to change the server/client log levels.

#6365

api/agent_test.go

client/monitor_endpoint.go

schmichael

Yay! Very excited about this. The framing might be obnoxious, but that can be done in a followup PR. I'm not quite through yet but will pick this up in the morning.

api/agent.go

api/agent_test.go

client/monitor_endpoint.go

command/agent/monitor/monitor.go

command/agent_monitor.go

nomad/client_agent_endpoint.go

api/agent.go

client/agent_endpoint.go

cgbaker

a few questions about locking in Monitor, but i'll leave it up to your judgement and check again if you have questions.

client/agent_endpoint.go

cgbaker · 2019-11-04T19:18:14Z

client/agent_endpoint_test.go

+	}
+}
+
+func TestMonitor_Monitor_ACL(t *testing.T) {


👍 💯 🏆

command/agent/monitor/monitor.go

cgbaker · 2019-11-04T20:03:15Z

command/agent/monitor/monitor.go

+				break LOOP
+			case <-time.After(d.droppedDuration):
+				d.Lock()
+				defer d.Unlock()


i'm thinking that this shouldn't be a defer, but instead an inline call to unlock after the if block

I think we want to hold onto the lock until after all the selects to ensure that a write doesn't steal the space we create to add the dropped message

cgbaker · 2019-11-04T20:06:22Z

command/agent/monitor/monitor.go

+				}
+			case <-stopCh:
+				d.Lock()
+				defer d.Unlock()


~~not sure whether this lock/unlock is needed, but it doesn't hurt anything.~~

not sure whether this is still true based on our personal conversation and other changes in flight

I think its unnecessary as well, will remove

if we are closing d.logCh here, then we need the lock to protect from other goroutines using this channel. if we aren't closing this channel, then i think we can drop the lock here.

This version of gziphandler includes a fix that fixes GzipResponseWriter to implement CloseNotifier nytimes/gziphandler#63

AgentMonitor is an endpoint to stream logs for a given agent. It allows callers to pass in a supplied log level, which may be different than the agents config allowing for temporary debugging with lower log levels. Pass in logWriter when setting up Agent

Queries /v1/agent/monitor and receives streaming logs from client

Adds nomad monitor command. Like consul monitor, this command allows you to stream logs from a nomad agent in real time with a a specified log level add endpoint tests Upgrade go-hclog to latest version The current version of go-hclog pads log prefixes to equal lengths so info becomes [INFO ] and debug becomes [DEBUG]. This breaks hashicorp/logutils/level.go Check function. Upgrading to the latest version removes this padding and fixes log filtering that uses logutils Check

multisink logger remove usage of logwriter

prefix output with proper spacing update gzip handler, adjust first byte flow to allow gzip handler bypass wip, first stab at wiring up rpc endpoint

Adds new package that can be used by client and server RPC endpoints to facilitate monitoring based off of a logger clean up old code small comment about write rm old comment about minsize rename to Monitor Removes connection logic from monitor command Keep connection logic in endpoints, use a channel to send results from monitoring use new multisink logger and interfaces small test for dropped messages update go-hclogger and update sink/intercept logger interfaces

underscores instead of dashes for query params

Addresses feedback around monitor implementation subselect on stopCh to prevent blocking forever. Set up a separate goroutine to check every 3 seconds for dropped messages. rename returned ch to avoid confusion

rm redundant lock wip to use framing wip switch to stream frames

rm extra new line fix lint errors return after close fix, simplify test

remove unused struct

fix typo command/agent/monitor/monitor.go Co-Authored-By: Chris Baker <[email protected]> Update command/agent/monitor/monitor.go Co-Authored-By: Chris Baker <[email protected]> address feedback, lock to prevent send on closed channel fix lock/unlock for dropped messages

command/agent/monitor/monitor.go

cgbaker

looks correct, i think there's some opportunity for simplification

command/agent/monitor/monitor.go

cgbaker · 2019-11-05T15:32:18Z

command/agent/monitor/monitor.go

+				}
+			case <-stopCh:
+				d.Lock()
+				defer d.Unlock()


if we are closing d.logCh here, then we need the lock to protect from other goroutines using this channel. if we aren't closing this channel, then i think we can drop the lock here.

cgbaker

need to unlock before quitting the dropped-goroutine

command/agent/monitor/monitor.go

cgbaker

👍 🚀

comments

comment, trigger build return length written

github-actions · 2023-01-27T02:16:55Z

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

drewbailey requested a review from schmichael October 15, 2019 20:48

drewbailey commented Oct 15, 2019

View reviewed changes

api/agent_test.go Outdated Show resolved Hide resolved

drewbailey commented Oct 15, 2019

View reviewed changes

client/monitor_endpoint.go Outdated Show resolved Hide resolved

drewbailey commented Oct 15, 2019

View reviewed changes

client/monitor_endpoint.go Outdated Show resolved Hide resolved

drewbailey force-pushed the f-nomad-monitor branch 2 times, most recently from 0b5b068 to 41b053f Compare October 25, 2019 14:53

drewbailey marked this pull request as ready for review October 25, 2019 18:32

drewbailey changed the title ~~[WIP] Nomad Monitor~~ Nomad Monitor Oct 25, 2019

schmichael reviewed Oct 29, 2019

View reviewed changes

drewbailey force-pushed the f-nomad-monitor branch from 4f0befc to cac0a6d Compare October 30, 2019 17:24

schmichael mentioned this pull request Oct 31, 2019

Safe defaults for agent file logging #6603

Closed

drewbailey force-pushed the f-nomad-monitor branch from 77c7a04 to 43ea91b Compare November 1, 2019 15:53

schmichael reviewed Nov 1, 2019

View reviewed changes

schmichael approved these changes Nov 1, 2019

View reviewed changes

drewbailey force-pushed the f-nomad-monitor branch 2 times, most recently from e385bc9 to b0a0cf0 Compare November 4, 2019 14:36

cgbaker reviewed Nov 4, 2019

View reviewed changes

api/agent.go Show resolved Hide resolved

cgbaker reviewed Nov 4, 2019

View reviewed changes

client/agent_endpoint.go Outdated Show resolved Hide resolved

cgbaker approved these changes Nov 4, 2019

View reviewed changes

drewbailey added 11 commits November 5, 2019 09:51

Update gziphandler to latest version

5e03fec

This version of gziphandler includes a fix that fixes GzipResponseWriter to implement CloseNotifier nytimes/gziphandler#63

Adds AgentMonitor Endpoint

91c0184

AgentMonitor is an endpoint to stream logs for a given agent. It allows callers to pass in a supplied log level, which may be different than the agents config allowing for temporary debugging with lower log levels. Pass in logWriter when setting up Agent

Add Agent Monitor to receive streaming logs

dc32864

Queries /v1/agent/monitor and receives streaming logs from client

Display error when remote side ended monitor

a828c92

multisink logger remove usage of logwriter

remove log_writer

1281997

prefix output with proper spacing update gzip handler, adjust first byte flow to allow gzip handler bypass wip, first stab at wiring up rpc endpoint

get local rpc endpoint working

890b8a4

enable json formatting, use queryoptions

e758930

new hclog with standardlogger intercept

2362008

use intercepting loggers for rpchandlers

3c0082f

drewbailey added 13 commits November 5, 2019 09:51

address feedback, use agent_endpoint instead of monitor

1176fc0

rename function, initialize log level better

6bf8617

underscores instead of dashes for query params

return 400 if invalid log_json param is given

873969c

Addresses feedback around monitor implementation subselect on stopCh to prevent blocking forever. Set up a separate goroutine to check every 3 seconds for dropped messages. rename returned ch to avoid confusion

fix deadlock issue, switch to frames envelope

58117c0

lock in sub select

f8eaf1f

rm redundant lock wip to use framing wip switch to stream frames

moving endpoints over to frames

9a96c10

monitor command takes no args

f74bd99

rm extra new line fix lint errors return after close fix, simplify test

allow more time for streaming message

8423ccf

remove unused struct

address feedback

676800f

move forwarded monitor request into helper

dc977dc

simplify assert message

4f618eb

address feedback, fix gauge metric name

bb2a7f4

drewbailey force-pushed the f-nomad-monitor branch from 995a23b to 620fba5 Compare November 5, 2019 14:52

cgbaker reviewed Nov 5, 2019

View reviewed changes

command/agent/monitor/monitor.go Show resolved Hide resolved

cgbaker approved these changes Nov 5, 2019

View reviewed changes

cgbaker suggested changes Nov 5, 2019

View reviewed changes

command/agent/monitor/monitor.go Outdated Show resolved Hide resolved

command/agent/monitor/monitor.go Outdated Show resolved Hide resolved

drewbailey requested a review from cgbaker November 5, 2019 16:22

cgbaker approved these changes Nov 5, 2019

View reviewed changes

drewbailey added 4 commits November 5, 2019 11:44

coordinate closing of doneCh, use interface to simplify callers

79411c5

comments

wireup plain=true|false query param

1585179

simplify logch goroutine

8ccb770

unlock before returning, no need for label

03f0aff

comment, trigger build return length written

drewbailey force-pushed the f-nomad-monitor branch from 65036cb to 03f0aff Compare November 5, 2019 16:44

drewbailey merged commit eb51141 into master Nov 5, 2019

drewbailey deleted the f-nomad-monitor branch November 5, 2019 17:42

drewbailey added this to the 0.10.2 milestone Nov 6, 2019

preetapan mentioned this pull request Nov 20, 2019

nomad monitor CLI #6365

Closed

github-actions bot locked as resolved and limited conversation to collaborators Jan 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nomad Monitor #6499

Nomad Monitor #6499

drewbailey commented Oct 15, 2019 •

edited

Loading

schmichael left a comment

cgbaker left a comment

cgbaker Nov 4, 2019

cgbaker Nov 4, 2019

drewbailey Nov 4, 2019

cgbaker Nov 4, 2019 •

edited

Loading

drewbailey Nov 4, 2019

cgbaker Nov 5, 2019

cgbaker left a comment

cgbaker Nov 5, 2019

cgbaker left a comment

cgbaker left a comment

github-actions bot commented Jan 27, 2023

Nomad Monitor #6499

Nomad Monitor #6499

Conversation

drewbailey commented Oct 15, 2019 • edited Loading

schmichael left a comment

Choose a reason for hiding this comment

cgbaker left a comment

Choose a reason for hiding this comment

cgbaker Nov 4, 2019

Choose a reason for hiding this comment

cgbaker Nov 4, 2019

Choose a reason for hiding this comment

drewbailey Nov 4, 2019

Choose a reason for hiding this comment

cgbaker Nov 4, 2019 • edited Loading

Choose a reason for hiding this comment

drewbailey Nov 4, 2019

Choose a reason for hiding this comment

cgbaker Nov 5, 2019

Choose a reason for hiding this comment

cgbaker left a comment

Choose a reason for hiding this comment

cgbaker Nov 5, 2019

Choose a reason for hiding this comment

cgbaker left a comment

Choose a reason for hiding this comment

cgbaker left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 27, 2023

drewbailey commented Oct 15, 2019 •

edited

Loading

cgbaker Nov 4, 2019 •

edited

Loading