Skip to content
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 1827852 - Extend start/stop time of a ping to millisecond precision #2456

Merged
merged 6 commits into from
Oct 19, 2023

Conversation

badboy
Copy link
Member

@badboy badboy commented Apr 26, 2023

@badboy badboy force-pushed the start-end-time-milliseconds branch 4 times, most recently from 8e16a02 to d03bb10 Compare June 28, 2023 12:58
Copy link
Member

@travis79 travis79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to update the docs to indicate the new precision.

@badboy badboy force-pushed the start-end-time-milliseconds branch from d03bb10 to c83d3cd Compare July 12, 2023 09:00
@badboy badboy force-pushed the start-end-time-milliseconds branch from c83d3cd to 8bc7031 Compare August 2, 2023 12:21
@badboy badboy force-pushed the start-end-time-milliseconds branch 5 times, most recently from 9cf3397 to 0f8775f Compare September 13, 2023 13:41
@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (c91af19) 33.33% compared to head (d1e3345) 33.33%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2456   +/-   ##
=======================================
  Coverage   33.33%   33.33%           
=======================================
  Files           1        1           
  Lines          42       42           
=======================================
  Hits           14       14           
  Misses         28       28           
Files Coverage Δ
glean-core/metrics.yaml 33.33% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@firefoxci-taskcluster
Copy link

Uh oh! Looks like an error! Details

Taskcluster-GitHub attempted to cancel previously created task groups with following scopes:

assume:repo:github.com/mozilla/glean:*, queue:seal-task-group:taskcluster-github/*, queue:cancel-task-group:taskcluster-github/*

Client ID static/taskcluster/github does not have sufficient scopes and is missing the following scopes:

queue:seal-task-group:glean-level-1/JhpvU9sKT9a6qKcKyJSSvg

This request requires the client to satisfy the following scope expression:

queue:seal-task-group:glean-level-1/JhpvU9sKT9a6qKcKyJSSvg

  • method: sealTaskGroup
  • errorCode: InsufficientScopes
  • statusCode: 403
  • time: 2023-10-19T11:52:25.800Z

@badboy badboy marked this pull request as ready for review October 19, 2023 11:53
@badboy badboy requested a review from a team as a code owner October 19, 2023 11:53
@badboy badboy requested review from travis79 and removed request for a team October 19, 2023 11:53
@badboy badboy force-pushed the start-end-time-milliseconds branch from 5c66891 to 61c73ad Compare October 19, 2023 12:01
@firefoxci-taskcluster
Copy link

Uh oh! Looks like an error! Details

Taskcluster-GitHub attempted to cancel previously created task groups with following scopes:

assume:repo:github.com/mozilla/glean:*, queue:seal-task-group:taskcluster-github/*, queue:cancel-task-group:taskcluster-github/*

Client ID static/taskcluster/github does not have sufficient scopes and is missing the following scopes:

queue:seal-task-group:glean-level-1/fH3ZnSm9QduWJn_L6FSDvg

This request requires the client to satisfy the following scope expression:

queue:seal-task-group:glean-level-1/fH3ZnSm9QduWJn_L6FSDvg

  • method: sealTaskGroup
  • errorCode: InsufficientScopes
  • statusCode: 403
  • time: 2023-10-19T12:01:44.315Z

@badboy badboy force-pushed the start-end-time-milliseconds branch from 61c73ad to d1e3345 Compare October 19, 2023 12:32
@firefoxci-taskcluster
Copy link

Uh oh! Looks like an error! Details

Taskcluster-GitHub attempted to cancel previously created task groups with following scopes:

assume:repo:github.com/mozilla/glean:*, queue:seal-task-group:taskcluster-github/*, queue:cancel-task-group:taskcluster-github/*

Client ID static/taskcluster/github does not have sufficient scopes and is missing the following scopes:

queue:seal-task-group:glean-level-1/JhpvU9sKT9a6qKcKyJSSvg

This request requires the client to satisfy the following scope expression:

queue:seal-task-group:glean-level-1/JhpvU9sKT9a6qKcKyJSSvg

  • method: sealTaskGroup
  • errorCode: InsufficientScopes
  • statusCode: 403
  • time: 2023-10-19T12:33:08.544Z

Copy link
Member

@travis79 travis79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@@ -29,6 +29,7 @@ In addition to those built-in metrics, the following metrics are added to the pi

| Name | Type | Description | Data reviews | Extras | Expiration | [Data Sensitivity](https://wiki.mozilla.org/Firefox/Data_Collection) |
| --- | --- | --- | --- | --- | --- | --- |
| glean.client.annotation.experimentation_id |[string](https://mozilla.github.io/glean/book/user/metrics/string.html) |An experimentation identifier derived and provided by the application for the purpose of experimentation enrollment. |[Bug 1848201](https://bugzilla.mozilla.org/show_bug.cgi?id=1848201#c5)||never |1 |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Don't we recommend folks use the Glean Dictionary for this stuff? (Non-blocking, just thinking out loud about filing a bug)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, we do. for others. We still have it and whether we should or not should go into a separate bug.

glean-core/metrics.yaml Show resolved Hide resolved
@@ -498,8 +500,6 @@ glean.error:
Only sent if the buffer ever overflows.

In Version 0 this reported the total number of tasks enqueued.
unit:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

o_O. Where did this come from? Ahh, cool! Even though unit has not meaning here, it still shows up in the Glean dictionary. Maybe this would be okay to leave as I kinda like the explicit documentation this metadata provides (though the description should do that, too) 🤔

No strong opinions on this, happy either way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was equally surprised!
We just disallowed this in glean_parser. So we would need to retract that decision, which would require a bug, a revert and a new release.

glean-core/rlb/src/lib.rs Show resolved Hide resolved
glean-core/src/core/mod.rs Show resolved Hide resolved
@badboy badboy merged commit 514cbbe into main Oct 19, 2023
33 checks passed
@badboy badboy deleted the start-end-time-milliseconds branch October 19, 2023 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants