Closed Bug 1350044 Opened 3 years ago Closed 3 years ago

Expose whether pingSender sent the ping in the telemetry corpus

Categories

(Cloud Services Graveyard :: Metrics: Pipeline, enhancement, P1)

enhancement

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
Hrm, looks like we might be getting these headers from bug 1333128 instead?
Depends on: 1333128
(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.
Whiteboard: [measurement:client:tracking]
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?
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)
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)
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: nobody → alessio.placitelli
(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
Points: --- → 3
Priority: -- → P1
Whiteboard: [measurement:client:tracking] → [measurement:client:tracking][Svc_Ops]
Whiteboard: [measurement:client:tracking][Svc_Ops] → [measurement:client:tracking][SvcOps]
I deployed this sprint's updates and verified on the CEP that at least one telemetry message contained "Fields[X-PingSender-Version]".
(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: 3 years ago
Resolution: --- → FIXED
Product: Cloud Services → Cloud Services Graveyard
You need to log in before you can comment on or make changes to this bug.