-
Notifications
You must be signed in to change notification settings - Fork 483
register available_managed_shm metric only for readout-proxy #13695
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
Conversation
|
REQUEST FOR PRODUCTION RELEASES: This will add The following labels are available |
davidrohr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, if @ktf could confirm that it will work.
I guess there is no good way to test it?
Or perhaps you can test with the stdout backend of the metrics, and then just check on the console that you get it only from readout-proxy?
| .sendInitialValue = true}}; | ||
|
|
||
| for (auto& metric : metrics) { | ||
| if (metric.metricId == (int)ProcessingStatsId::AVAILABLE_MANAGED_SHM_BASE + (runningWorkflow.shmSegmentId % 512) && spec.name.compare("readout-proxy") != 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "readout-proxy" the only name we have?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is the only name we use in the online processing, and we only display it for this task in Grafana. The calibration workflows have different input proxy names each, and (at least so far) we don't look at their shm.
|
Probably the correct way of doing this would be to have the metric specification as part of a service and allow devices to add their own customisations... I would say good enough for now (modulo the comment on the name) and we can have a better fix later. |
|
So I tested it locally with the metrics display in the GUI, using a simple workflow and replacing the check for I also used the monitoring stdout backend with the o2-dpl-raw-proxy, using two different names: |
this provides a quick fix for https://its.cern.ch/jira/browse/O2-4234
now there is the comment about the ugliness of the condition in this place. @ktf to you have a better general solution for this in mind? At the moment, I can only think of more ugly ways to decouple the sending of the other metrics from the
available_managed_shmfor thereadout-proxyonly...