1350044 - Expose whether pingSender sent the ping in the telemetry corpus

Status: RESOLVED FIXED

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

Description

[Chris H-C :chutten|RustWeek (Back May 25)]

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

Comment 1

[Chris H-C :chutten|RustWeek (Back May 25)]

Hrm, looks like we might be getting these headers from bug 1333128 instead?

Comment 2

[Alessio Placitelli [:Dexter]]

(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.

Comment 3

[Alessio Placitelli [:Dexter]]

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?

Comment 4

[Alessio Placitelli [:Dexter]]

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?

Comment 5

[Mark Reid [: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

Comment 6

[Chris H-C :chutten|RustWeek (Back May 25)]

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.

Comment 7

[Alessio Placitelli [:Dexter]]

(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

Comment 8

[Wesley Dawson [:whd]]

I deployed this sprint's updates and verified on the CEP that at least one telemetry message contained "Fields[X-PingSender-Version]".

Comment 9

[Alessio Placitelli [:Dexter]]

(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!