Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server/status: add NodeStatus.{Args,Env} #14076

Merged

Conversation

petermattis
Copy link
Collaborator

@petermattis petermattis commented Mar 10, 2017

Fixes #14065


This change is Reviewable

@petermattis petermattis force-pushed the pmattis/node-status-args branch from 00cef05 to dc58da6 Compare March 10, 2017 19:49
@tamird
Copy link
Contributor

tamird commented Mar 10, 2017

As always, it's good to dereference the issue into the commit message.

:lgtm:


Reviewed 9 of 9 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.


pkg/server/status/recorder_test.go, line 337 at r1 (raw file):

	// Make sure there is at least one environment variable that will be
	// reported.
	if err := os.Setenv("GOGC", "100"); err != nil {

clean this up after the test?


Comments from Reviewable

Add the COCKROACH_* and GO* env vars and the command line flags to
NodeStatus so that they will be collected by the debug zip command.

Fixes cockroachdb#14065
@petermattis petermattis force-pushed the pmattis/node-status-args branch from dc58da6 to f6e2313 Compare March 10, 2017 20:16
@petermattis
Copy link
Collaborator Author

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


pkg/server/status/recorder_test.go, line 337 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

clean this up after the test?

Seems like more trouble than its worth. The variable is only consulted by the Go runtime at startup.


Comments from Reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants