Open Bug 1520739 Opened 9 months ago Updated Yesterday

Show measurement units

Categories

(Tree Management :: Perfherder, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: igoldan, Unassigned, Mentored)

References

(Depends on 4 open bugs, Blocks 2 open bugs)

Details

(Whiteboard: [lang=py][lang=js])

We have to show measurement units in alerts, otherwise developers get easily confused and don't know how to interpret the performance graphs.

Mentor: igoldan
Priority: -- → P3
Duplicate of this bug: 1519610

I would like to work on this bug

Keywords: good-first-bug
Whiteboard: [lang=py][lang=js]

(In reply to hamzah18051 from comment #2)

I would like to work on this bug

Looking close into this, we realized it's not really a good first bug.

Duplicate of this bug: 1577104

We should extend this to all views, so once we have the unit data we can include it in graph, compare, alerts, etc.

igoldan: As this involves the graphs view, do you think we could include this in our Q4 work? What's involved here? I imagine we could add a 'unit' field in the schema, which could be bytes, seconds, etc. Tests would then need to provide this information, allowing us to format the display of the results accordingly (defaulting to the current behaviour).

Flags: needinfo?(igoldan)

(In reply to Dave Hunt [:davehunt] [he/him] ⌚BST from comment #5)

We should extend this to all views, so once we have the unit data we can include it in graph, compare, alerts, etc.

igoldan: As this involves the graphs view, do you think we could include this in our Q4 work?

Yes, we can include this into Q4.

What's involved here? I imagine we could add a 'unit' field in the schema, which could be bytes, seconds, etc. Tests would then need to provide this information, allowing us to format the display of the results accordingly (defaulting to the current behaviour).

I believe this is the indeed the way to proceed. Some of the tests already provide a measurement unit (the schema allows this).
After updating the schema, we'll need to add measurement units to those that don't. Likely, we'll sync with some of the test owners.

The tricky part will be Perfherder's ingestion pipeline. We need to make it aware of measurement units. At this moment, I'm not sure whether this new field should be part of a perf signature. Basically, what I'm asking is: if someone updates the measurement unit for a test, should we treat that as a whole new set of data?

After we address this problem, what'll be left are the actual UI displays of this unit.

Flags: needinfo?(igoldan)

(In reply to Ionuț Goldan [:igoldan], Performance Sheriff from comment #6)

The tricky part will be Perfherder's ingestion pipeline. We need to make it aware of measurement units. At this moment, I'm not sure whether this new field should be part of a perf signature. Basically, what I'm asking is: if someone updates the measurement unit for a test, should we treat that as a whole new set of data?

Do you have an argument for including it in the signature? Initially we'll only be adding unit metadata, but even if we were changing a unit (let's say for some reason we changed bytes to seconds) then this would either be a correction or a significant change to the type of result we're submitting, in which case we'd expect to see an alert, which will point to the commit that modified the unit value.

(In reply to Dave Hunt [:davehunt] [he/him] ⌚BST from comment #7)

[...] but even if we were changing a unit (let's say for some reason we changed bytes to seconds) then this would either be a correction or a significant change to the type of result we're submitting, in which case we'd expect to see an alert, which will point to the commit that modified the unit value.

Indeed. These are valid points for not including the measurement unit into the signatures.

You need to log in before you can comment on or make changes to this bug.