Closed
Bug 1261865
Opened 8 years ago
Closed 8 months ago
Add commit hook to reject bucket count changes in Histograms.json
Categories
(Toolkit :: Telemetry, defect, P4)
Toolkit
Telemetry
Tracking
()
RESOLVED
WONTFIX
Tracking | Status | |
---|---|---|
firefox48 | --- | affected |
People
(Reporter: gfritzsche, Unassigned)
References
Details
(Whiteboard: [measurement:client])
(In reply to Roberto Agostino Vitillo (:rvitillo) from bug 1261530, comment #3) > We have considered histograms to be immutable until now and I would like > this to continue to be the case. Since several systems depend on that > assumption (cerberus, medusa, moztelemetry, mozaggregator, > telemetry-batch-view and possibly others), could we add some checks on the > client to avoid this from happening again? The alternative is to add checks > on all the systems downstream that depend on that assumptions. I don't see us covering that easily in a client-build, but we could look into a commit hook that enforces this on changes to Histograms.json.
Reporter | ||
Comment 1•8 years ago
|
||
The idea would be to add a commit hook that: * only looks at changes to Histograms.json * (it could look only at modifications, not additions or deletions) * parses the old & new Histograms.json * for each histogram entry that is in both in the old & new Histograms.json, checks that the following fields didn't change: * kind, keyed, low, high, n_buckets, n_values
Reporter | ||
Comment 2•8 years ago
|
||
... and if they changed, rejects the commit with a descriptive message. Does that sound good to you Roberto?
Flags: needinfo?(rvitillo)
Reporter | ||
Comment 4•8 years ago
|
||
gps, does that look like a feasible commit hook? If yes, is that something you could mentor or do you know who could?
Flags: needinfo?(gps)
Comment 5•8 years ago
|
||
Yes, this is feasible. See https://hg.mozilla.org/hgcustom/version-control-tools/file/tip/hghooks/mozhghooks/prevent_idl_change_without_uuid_bump.py and https://hg.mozilla.org/hgcustom/version-control-tools/file/tip/hghooks/tests/test-prevent-idl-change-without-uuid-bump.t for a similar hook and test that looks at file content. One thing I'll warn you against for implementing pre-commit hooks is that if you ever change the format of the histogram file you'll need to update the hook. And this change will need to "ride the trains" which is a bit difficult in practice since hooks don't ride the trains since they aren't defined in the repo.
Flags: needinfo?(gps)
Reporter | ||
Updated•7 years ago
|
Priority: P3 → P4
Updated•2 years ago
|
Severity: normal → S3
Updated•8 months ago
|
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•