Closed
Bug 1350044
Opened 7 years ago
Closed 7 years ago
Expose whether pingSender sent the ping in the telemetry corpus
Categories
(Cloud Services Graveyard :: Metrics: Pipeline, enhancement, P1)
Cloud Services Graveyard
Metrics: Pipeline
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: chutten, Assigned: Dexter)
References
Details
(Whiteboard: [measurement:client:tracking][SvcOps])
In bug 1336360 we're adding a couple of headers[1] to positively identify that pingSender was the source of a ping. That a ping came from pingSender would be useful to have in the Telemetry dataset for a couple of reasons... not exhaustively: 1) Duplicate detection (did a duplication happen because both pingSender and gecko sent the same docid?) 2) Success Rates (how often does pingSender successfully send a ping, and how often does it fail) [1] User-Agent: pingsender/1.0 and X-PingSender-Version: 1.0
Reporter | ||
Comment 1•7 years ago
|
||
Hrm, looks like we might be getting these headers from bug 1333128 instead?
Depends on: 1333128
Assignee | ||
Comment 2•7 years ago
|
||
(In reply to Chris H-C :chutten from comment #1) > Hrm, looks like we might be getting these headers from bug 1333128 instead? +Mark Yeah, they weren't really relevant to bug 1336360 either, so I decided to land them sooner in bug 1333128, to unblock any pipeline work (if needed) and allow some time for this to stabilise on Nightly.
Assignee | ||
Updated•7 years ago
|
Whiteboard: [measurement:client:tracking]
Assignee | ||
Comment 3•7 years ago
|
||
For context, the rationale behind adding the "X-" prefixed version header. (In reply to Mark Reid [:mreid] from bug 1336360 comment #36) > It might still be a good idea to add an "X-" prefixed header for just this > new piece of data. We're not currently storing the user agent string, and > IIRC it can be quite long and could require storing a fair chunk of extra > data (that's mostly repeated in the data contents already). Adding a > "X-PingSender-Version: 1.0" header or similar could express the same info > more succinctly. What do you think?
Assignee | ||
Comment 4•7 years ago
|
||
Mark, is there anything that needs to be done to expose the additional headers we're sending from the ping sender to Spark notebooks/datasets?
Flags: needinfo?(mreid)
Comment 5•7 years ago
|
||
Yes, we need to make two small changes to add this information to the Telemetry data. I am inclined to only add the "X-PingSender-Version" header, unless there is a compelling reason to capture the User Agent. 1. Add the field(s) to the decoder. Other incoming headers are captured at [1]. 2. Add the headers to the ingest code so it's available to the decoder. This is done using the 'moz_ingest_header' config param as described at [2]. The actual changes need to be made to the puppet config at [3]. [1] https://github.com/mozilla-services/lua_sandbox_extensions/blob/master/moz_telemetry/io_modules/decoders/moz_telemetry/ping.lua#L474-L476 [2] https://github.com/mozilla-services/nginx_moz_ingest [3] https://github.com/mozilla-services/puppet-config/blob/master/pipeline/modules/pipeline/templates/edge/nginx.conf.erb#L25-L30
Flags: needinfo?(mreid)
Reporter | ||
Comment 6•7 years ago
|
||
As much as I'm curious about how many of our users are masquerading themselves UA-wise, right now the important piece of information is "Was this sent by pingsender?" (and in the near future "which version of pingSender") so having a meta/pingSenderVersion field would be sufficient for the use cases I've heard.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → alessio.placitelli
Assignee | ||
Comment 7•7 years ago
|
||
(In reply to Mark Reid [:mreid] from comment #5) > Yes, we need to make two small changes to add this information to the > Telemetry data. I am inclined to only add the "X-PingSender-Version" header, > unless there is a compelling reason to capture the User Agent. Thanks Mark! > 1. Add the field(s) to the decoder. Other incoming headers are captured at > [1]. https://github.com/mozilla-services/lua_sandbox_extensions/pull/106 > 2. Add the headers to the ingest code so it's available to the decoder. This > is done using the 'moz_ingest_header' config param as described at [2]. The > actual changes need to be made to the puppet config at [3]. https://github.com/mozilla-services/puppet-config/pull/2539
Updated•7 years ago
|
Points: --- → 3
Priority: -- → P1
Updated•7 years ago
|
Whiteboard: [measurement:client:tracking] → [measurement:client:tracking][Svc_Ops]
Updated•7 years ago
|
Whiteboard: [measurement:client:tracking][Svc_Ops] → [measurement:client:tracking][SvcOps]
Comment 8•7 years ago
|
||
I deployed this sprint's updates and verified on the CEP that at least one telemetry message contained "Fields[X-PingSender-Version]".
Assignee | ||
Comment 9•7 years ago
|
||
(In reply to Wesley Dawson [:whd] from comment #8) > I deployed this sprint's updates and verified on the CEP that at least one > telemetry message contained "Fields[X-PingSender-Version]". Thanks Wesley!
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Cloud Services → Cloud Services Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•