Skip to content

Conversation

@ehellbar
Copy link
Collaborator

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_shm for the readout-proxy only...

@github-actions
Copy link
Contributor

REQUEST FOR PRODUCTION RELEASES:
To request your PR to be included in production software, please add the corresponding labels called "async-" to your PR. Add the labels directly (if you have the permissions) or add a comment of the form (note that labels are separated by a ",")

+async-label <label1>, <label2>, !<label3> ...

This will add <label1> and <label2> and removes <label3>.

The following labels are available
async-2023-pbpb-apass4
async-2023-pp-apass4
async-2024-pp-apass1
async-2022-pp-apass7
async-2024-pp-cpass0
async-2024-PbPb-cpass0
async-2024-PbPb-apass1
async-2024-ppRef-apass1

Copy link
Collaborator

@davidrohr davidrohr left a 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) {
Copy link
Member

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?

Copy link
Collaborator Author

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.

@ktf
Copy link
Member

ktf commented Nov 18, 2024

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.

@ehellbar
Copy link
Collaborator Author

ehellbar commented Nov 21, 2024

So I tested it locally with the metrics display in the GUI, using a simple workflow and replacing the check for readout-proxy with another task name.

I also used the monitoring stdout backend with the o2-dpl-raw-proxy, using two different names:

o2-dpl-raw-proxy  --proxy-name readout-proxy --driver-client-backend "stdout://" -b > output_proxy-name_readout-proxy.log
grep -nr '\[METRIC\]' output_proxy-name_readout-proxy.log  | wc -l
      31
grep -nr '\[METRIC\] available_managed_shm' output_proxy-name_readout-proxy.log  | wc -l
       1
o2-dpl-raw-proxy  --proxy-name dummy-proxy --driver-client-backend "stdout://" -b > output_proxy-name_dummy-proxy.log
grep -nr '\[METRIC\]' output_proxy-name_dummy-proxy.log  | wc -l
      30
grep -nr '\[METRIC\] available_managed_shm' output_proxy-name_dummy-proxy.log  | wc -l
       0

@ehellbar ehellbar marked this pull request as ready for review November 22, 2024 12:17
@ehellbar ehellbar requested a review from a team as a code owner November 22, 2024 12:17
@davidrohr davidrohr merged commit 0c73418 into AliceO2Group:dev Nov 22, 2024
15 of 17 checks passed
@ehellbar ehellbar deleted the dev-metrics branch November 25, 2024 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants