-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug 1697342 - Support Glean.js pings in Rally decoder #1617
Conversation
ingestion-beam/src/main/java/com/mozilla/telemetry/decoder/DecryptJWE.java
Outdated
Show resolved
Hide resolved
ingestion-beam/src/main/java/com/mozilla/telemetry/decoder/DecryptJWE.java
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
With a clearer head, I found out that the ciphertext was out of date. Everything works as expected. |
fe347bc
to
afa0b07
Compare
Codecov Report
@@ Coverage Diff @@
## master #1617 +/- ##
=============================================
- Coverage 86.40% 73.69% -12.72%
+ Complexity 730 611 -119
=============================================
Files 106 80 -26
Lines 4340 3406 -934
Branches 401 369 -32
=============================================
- Hits 3750 2510 -1240
- Misses 466 796 +330
+ Partials 124 100 -24
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
ingestion-beam/src/main/java/com/mozilla/telemetry/decoder/rally/DecryptRallyPayloads.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing blocking here, but I'd like to see less responsibility encoded directly in Decoder.java
and left a few other comments about improving clarity for future readers. Mark me for review again when you've had a chance to respond, and I think we'll be good to go.
ingestion-beam/src/main/java/com/mozilla/telemetry/Decoder.java
Outdated
Show resolved
Hide resolved
ingestion-beam/src/main/java/com/mozilla/telemetry/decoder/rally/DecryptPayloads.java
Outdated
Show resolved
Hide resolved
ingestion-beam/src/main/java/com/mozilla/telemetry/Decoder.java
Outdated
Show resolved
Hide resolved
ingestion-beam/src/main/java/com/mozilla/telemetry/decoder/rally/DecryptRallyPayloads.java
Outdated
Show resolved
Hide resolved
ingestion-beam/src/main/java/com/mozilla/telemetry/decoder/rally/DecryptRallyPayloads.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor looks great. Left a few more non-blocking suggestions for further clarity.
ingestion-beam/src/main/java/com/mozilla/telemetry/decoder/rally/DecryptPayloads.java
Outdated
Show resolved
Hide resolved
ingestion-beam/src/main/java/com/mozilla/telemetry/Decoder.java
Outdated
Show resolved
Hide resolved
Alright, I'm going to merge this in. Thanks for the review @jklukas! |
bug link
This reuses much of the code from the DecryptAetIdentifiers and DecryptPioneerPayloads package.