1605097 - Persist HTTP headers for pending pings

Status: RESOLVED FIXED

(Data Platform and Tools :: Glean: SDK, defect, P1)

Description

[Beatriz Rizental [:brizental]]

From a comment in from [:Dexter] https://docs.google.com/document/d/1YEhrudNl5aHBnht3FCjT66Bj58ckDw3jO-_V56mnqHs/edit?usp=sharing:

The debug tag is one of the headers that need to be persisted (pings can fail to upload and we still want to send them to the debug endpoint next time) :) The date one, for example, doesn't need to be persisted (it's added on the fly when uploading)

Comment 1

[Beatriz Rizental [:brizental]]

Currently there is only one header which we need to persist, the X-Debug-ID header.

Soon though there will be more headers that will need persisting, see Bug 1634064. Therefore it is best to create a system for persisting these headers that will be fit for persisting as many as necessary.

I propose adding an optional new line to the ping file contents, with a JSON object containing all the custom headers. In case the ping does not have any custom headers the third line is just not added to the file, thus removing the need for any kind of migration for current ping file format.

What do you think [:janerik], [:Dexter]?

Comment 2

[Alessio Placitelli [:Dexter]]

(In reply to Beatriz Rizental from comment #1)

What do you think [:janerik], [:Dexter]?

I like the approach. Since we're here, we might as well include a general "metadata" section on the third line, to make it easy to extend in the future, if needed. For example, the JSON object could be something like.

{
  "persistedHeaders": [
    {
      "name": "X-Debug-ID",
      "value": ... 
    },
    ..
  ]
}

This way, if we'd ever want to add something more than persisted headers, we'd just add a new section to this jSON object.

Comment 3

[Jan-Erik Rediger [:janerik]]

Can we currently think of any other information we might need to add in the future?
If no, then it sounds fine to be, even if a bit adhoc.
If yes, we might want to reconsider the file format and use something less adhoc.

At the very least we should have explicit tests for these of course.

Comment 4

[Jan-Erik Rediger [:janerik]]

(In reply to Alessio Placitelli [:Dexter] from comment #2)

(In reply to Beatriz Rizental from comment #1)

What do you think [:janerik], [:Dexter]?

I like the approach. Since we're here, we might as well include a general "metadata" section on the third line, to make it easy to extend in the future, if needed. For example, the JSON object could be something like.

This way, if we'd ever want to add something more than persisted headers, we'd just add a new section to this jSON object.

If we do this let's not forget documenting the format properly (bikeshedding upcoming, I for one think we might not need the "name" / "value" part and the "persisted" is also superfluous)

Comment 5

[Alessio Placitelli [:Dexter]]

(In reply to Jan-Erik Rediger [:janerik] from comment #4)

(In reply to Alessio Placitelli [:Dexter] from comment #2)

(In reply to Beatriz Rizental from comment #1)

What do you think [:janerik], [:Dexter]?

I like the approach. Since we're here, we might as well include a general "metadata" section on the third line, to make it easy to extend in the future, if needed. For example, the JSON object could be something like.

This way, if we'd ever want to add something more than persisted headers, we'd just add a new section to this jSON object.

If we do this let's not forget documenting the format properly (bikeshedding upcoming, I for one think we might not need the "name" / "value" part and the "persisted" is also superfluous)

Heheh, fair point, I'm fine with dropping the above and go with your recommendation ;)

Comment 6

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/glean/pull/1042