Closed Bug 1381653 Opened 3 years ago Closed 1 year ago

Add SANDBOX_REJECTED_SYSCALLS histogram to Main Summary

Categories

(Data Platform and Tools :: Datasets: Main Summary, enhancement, P1)

Unspecified
Linux
enhancement
Points:
1

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jld, Assigned: frank)

References

Details

(Whiteboard: sb+)

Attachments

(1 file)

SANDBOX_REJECTED_SYSCALLS is a (hopefully) very sparsely populated histogram used by Linux Firefox when system calls are rejected by a sandbox policy.  We'd like to have it added to Main Summary so we can see the entire set of reported keys.
Whiteboard: sb? → sblc2
Whiteboard: sblc2 → sb?
Flags: needinfo?(jld)
Priority: -- → P2
Whiteboard: sb? → sb+
Should I try to write the patch for this myself, or should this be assigned to someone who knows the relevant code?
Flags: needinfo?(jld) → needinfo?(mreid)
You could definitely submit a PR for this yourself if you like. There is a whitelist of histograms that are included in main summary:

https://github.com/mozilla/telemetry-batch-view/blob/master/src/main/scala/com/mozilla/telemetry/views/MainSummaryView.scala#L22

Simply add this one (in alphabetic order, please) and we'll be good to go.
Flags: needinfo?(mreid)
Assignee: nobody → jld
Priority: P2 → P1
:jld has sent a PR for this here:
https://github.com/mozilla/telemetry-batch-view/pull/299
Points: --- → 1
This is blocked because we don't currently support "count" histograms in Main Summary.

There are 2 options, as I see it:
1. Add this as a custom field in main_summary (don't rely on the histogram whitelist).
2. Lift the restriction excluding "count" histograms.

I'm leaning more towards the 2nd option, but having this type of histogram actually appear as a numeric field in the table.

Thoughts?
Flags: needinfo?(fbertsch)
The other important piece of this conversation is that eventually, all count histograms should be implemented as scalars, which will automatically show up in main_summary. Because of that, I think we should do the following:

1. Add count histograms, but not as a special case (i.e. still a MAP)
2. Make sure there is a bug out for converting this to a scalar

My rationale is that the special case is extra code and work for something we already do - record scalars :)

If it's wanted to be a scalar field as-is, this should be a regular top-level field in main_summary with custom extraction code.
Flags: needinfo?(fbertsch)
Priority: P1 → P2
We have support for count histograms now, so this should (finally!) be easy.
Assignee: jld → fbertsch
Priority: P2 → P1
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.