Closed Bug 665805 Opened 9 years ago Closed 9 years ago

Adjust telemetry to work with official metrics server

Categories

(Toolkit :: Telemetry, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla7

People

(Reporter: taras.mozilla, Unassigned)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached patch fixes (obsolete) — Splinter Review
This adjusts the protocol to include content-type: application/json. I also noticed that 404s were treated as success and histogram[0] was reporting the values.
Updated the testcase accordingly.
Attachment #540633 - Flags: review?(dtownsend)
This also strips {} from uuids to make the metrics server happy
Attachment #540640 - Flags: review?(dtownsend)
Comment on attachment 540633 [details] [diff] [review]
fixes

Review of attachment 540633 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/telemetry/TelemetryPing.js
@@ +252,5 @@
>      let startTime = new Date()
>  
> +    function finishRequest(status_code) {
> +      // Accept codes 200, 205, etc
> +      let success = 200 == Math.round(status_code / 100) * 100;

Please use request.channel.requestSucceeded

::: toolkit/components/telemetry/tests/unit/test_TelemetryPing.js
@@ +45,5 @@
>    createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
>    httpserver.start(4444);
>  
> +  Services.obs.addObserver(nonexistentServerObserver, "telemetry-test-xhr-complete", false);
> +  telemetry_ping("http://0.0.0.0");

I'm concerned that this will still touch the network, can you just use 127.0.0.1 and run this before the test server has started?
Attachment #540633 - Flags: review?(dtownsend) → review+
Comment on attachment 540640 [details] [diff] [review]
Include sum in serialized histograms

Review of attachment 540640 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #540640 - Flags: review?(dtownsend) → review+
Attached patch fixesSplinter Review
Addressed review comments, carrying over r+
Attachment #540633 - Attachment is obsolete: true
Attachment #540899 - Flags: review+
Whiteboard: [inbound]
Target Milestone: --- → mozilla7
Whiteboard: [inbound]
You need to log in before you can comment on or make changes to this bug.