Skip to content
This repository has been archived by the owner. It is now read-only.

fix(tests): add a way to disable client metrics stderr #3209

Merged
merged 1 commit into from Nov 13, 2015
Merged

Conversation

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Oct 21, 2015

Fixes #3158

@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Oct 21, 2015

@@ -48,6 +48,13 @@ var conf = module.exports = convict({
default: path.resolve(__dirname, '..', '..', 'cert.pem'),
doc: 'The location of the SSL certificate in pem format'
},
client_metrics: {
enabled: {

This comment has been minimized.

@shane-tomlinson

shane-tomlinson Oct 22, 2015
Member

client_metrics.enabled implies a global on/off switch of client metrics, whereas this flag only enables/disables output to stderr. Maybe this should be something like:

client_metrics: {
  stderr_collector_enabled: {
     ...
  },
  statsd_collector_enabled: {
    ...
  }
}

I could also see something like:

client_metrics: {
  collectors: {
     default: [ 'ga', 'stderr', 'statsd' ],
     doc: 'Where to send client metrics'
  }
}
@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Oct 30, 2015

shane-tomlinson was assigned by rfk 4 hours ago

I left my comments, back to @vladikoff

@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Nov 3, 2015

@shane-tomlinson I updated this to be more consistent with the "DISABLE_..." options we already have. Also added some tests. If we need to mute statsd and other logging we can add that later if required, following the same pattern.

@vladikoff vladikoff force-pushed the i3158 branch 3 times, most recently from fc5975c to 839ba0e Nov 3, 2015
@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Nov 4, 2015

uh metrics-unit keeps failing on node 0.10, will need to check what is going on, otherwise should be ready.

@vladikoff vladikoff force-pushed the i3158 branch 2 times, most recently from 088f183 to a70de55 Nov 12, 2015
@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Nov 12, 2015

Updated and should be ready!

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Nov 13, 2015

https://travis-ci.org/mozilla/fxa-content-server/jobs/90769425 No more client metrics! Now if we could just get rid of all this cruft:

[2015-11-12 16:49:54] bid.ccverifier.INFO: compute cluster: spawned new worker process (3882) 1/20 processes running
[2015-11-12 16:49:54] bid.ccverifier.DEBUG: compute cluster: passing compute job to process 3882
[2015-11-12 16:49:54] bid.ccverifier.INFO: compute cluster: spawned new worker process (3884) 2/20 processes running
[2015-11-12 16:49:54] bid.ccverifier.DEBUG: compute cluster: passing compute job to process 3884
[2015-11-12 16:49:55] bid.v2.INFO: assertion_verification_time 334
[2015-11-12 16:49:55] bid.server.INFO: 127.0.0.1 - - [Thu, 12 Nov 2015 16:49:55 GMT] "POST /v2 HTTP/1.1" 200 339 "-" "-"
[2015-11-12 16:49:55] bid.v2.INFO: verify { result: 'success', rp: 'http://127.0.0.1:9010' }
[2015-11-12 16:49:55] bid.ccverifier.DEBUG: compute cluster: process 3882 completed work in 0.35s
[2015-11-12 16:49:55] bid.v2.INFO: assertion_verification_time 344
[2015-11-12 16:49:55] bid.server.INFO: 127.0.0.1 - - [Thu, 12 Nov 2015 16:49:55 GMT] "POST /v2 HTTP/1.1" 200 339 "-" "-"
[2015-11-12 16:49:55] bid.v2.INFO: verify { result: 'success', rp: 'http://127.0.0.1:9010' }
[2015-11-12 16:49:55] bid.ccverifier.DEBUG: compute cluster: process 3884 completed work in 0.33s
[2015-11-12 16:49:55] bid.ccverifier.DEBUG: compute cluster: passing compute job to process 3882
[2015-11-12 16:49:55] bid.v2.INFO: assertion_verification_time 75
[2015-11-12 16:49:55] bid.server.INFO: 127.0.0.1 - - [Thu, 12 Nov 2015 16:49:55 GMT] "POST /v2 HTTP/1.1" 200 339 "-" "-"
[2015-11-12 16:49:55] bid.v2.INFO: verify { result: 'success', rp: 'http://127.0.0.1:9010' }
[2015-11-12 16:49:55] bid.ccverifier.DEBUG: compute cluster: process 3882 completed work in 0.08s
[2015-11-12 16:49:55] bid.ccverifier.DEBUG: compute cluster: passing compute job to process 3882
[2015-11-12 16:49:55] bid.v2.INFO: assertion_verification_time 63
[2015-11-12 16:49:55] bid.server.INFO: 127.0.0.1 - - [Thu, 12 Nov 2015 16:49:55 GMT] "POST /v2 HTTP/1.1" 200 339 "-" "-"
[2015-11-12 16:49:55] bid.v2.INFO: verify { result: 'success', rp: 'http://127.0.0.1:9010' }
[2015-11-12 16:49:55] bid.ccverifier.DEBUG: compute cluster: process 3882 completed work in 0.07s
shane-tomlinson pushed a commit that referenced this pull request Nov 13, 2015
fix(tests): add a way to disable client metrics stderr

r=@shane-tomlinson
@shane-tomlinson shane-tomlinson merged commit ace5046 into master Nov 13, 2015
4 checks passed
4 checks passed
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 99.15%
Details
@shane-tomlinson shane-tomlinson deleted the i3158 branch Nov 13, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants