You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "uds5501 (via GitHub)" <gi...@apache.org> on 2025/07/29 10:12:21 UTC
[PR] Remove stacktraces for RE failures on (druid)
uds5501 opened a new pull request, #18338:
URL: https://github.com/apache/druid/pull/18338
Removes stack traces for `RE` exception thrown when calling `discover()` on a misconfigured cpu discoverer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Remove stacktraces for RE failures on (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #18338:
URL: https://github.com/apache/druid/pull/18338#discussion_r2239383463
##########
processing/src/main/java/org/apache/druid/java/util/metrics/cgroups/Cpu.java:
##########
@@ -77,7 +77,7 @@ public CpuMetrics snapshot()
}
}
catch (IOException | RuntimeException ex) {
- LOG.error(ex, "Unable to fetch cpu snapshot");
+ LOG.noStackTrace().error(ex, "Unable to fetch cpu snapshot");
Review Comment:
This should be `warn`. Error is reserved for serious errors, but this isn't one, it just means that some metrics won't be reported. Btw, the error message should have more detail about the impact. It should mention that this means that `cgroup` metrics will not be emitted. Such as:
```
Unable to fetch CPU snapshot. Cgroup metrics will not be emitted.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Remove stacktraces for RE failures on (druid)
Posted by "uds5501 (via GitHub)" <gi...@apache.org>.
uds5501 commented on PR #18338:
URL: https://github.com/apache/druid/pull/18338#issuecomment-3133151958
Thanks for the review @gianm , I have accommodated the updates.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Remove stacktraces for RE failures on (druid)
Posted by "uds5501 (via GitHub)" <gi...@apache.org>.
uds5501 commented on code in PR #18338:
URL: https://github.com/apache/druid/pull/18338#discussion_r2241947027
##########
processing/src/main/java/org/apache/druid/java/util/metrics/cgroups/Cpu.java:
##########
@@ -77,7 +77,7 @@ public CpuMetrics snapshot()
}
}
catch (IOException | RuntimeException ex) {
- LOG.error(ex, "Unable to fetch cpu snapshot");
+ LOG.noStackTrace().error(ex, "Unable to fetch cpu snapshot");
Review Comment:
I don't think that would work, within `CgroupV2CpuMonitor` is supposed to use `ProcCgroupV2Discoverer` , that itself does not seem to implement the `discover(String cgroup)` completely . The `cgroup` is not being used in the [function](https://github.com/apache/druid/blob/d9f0af84af0d5ac85abd767777aa4c68c8640b23/processing/src/main/java/org/apache/druid/java/util/metrics/cgroups/ProcCgroupV2Discoverer.java#L45) anywhere so I am not confident that it's complete in itself.
Additionally `CgroupV2CpuMonitor` is marked `experimental` so I don't think we should be switching it internally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Remove stacktraces for RE failures on (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #18338:
URL: https://github.com/apache/druid/pull/18338#discussion_r2240483050
##########
processing/src/main/java/org/apache/druid/java/util/metrics/cgroups/Cpu.java:
##########
@@ -77,7 +77,7 @@ public CpuMetrics snapshot()
}
}
catch (IOException | RuntimeException ex) {
- LOG.error(ex, "Unable to fetch cpu snapshot");
+ LOG.noStackTrace().error(ex, "Unable to fetch cpu snapshot");
Review Comment:
Do you know if it is possible to detect which one is present and switch into the right one automatically? That seems better than trying to write an error message that guides people to make the right config change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Remove stacktraces for RE failures on (druid)
Posted by "cryptoe (via GitHub)" <gi...@apache.org>.
cryptoe merged PR #18338:
URL: https://github.com/apache/druid/pull/18338
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Remove stacktraces for RE failures on (druid)
Posted by "cryptoe (via GitHub)" <gi...@apache.org>.
cryptoe commented on code in PR #18338:
URL: https://github.com/apache/druid/pull/18338#discussion_r2239417044
##########
processing/src/main/java/org/apache/druid/java/util/metrics/cgroups/Cpu.java:
##########
@@ -77,7 +77,7 @@ public CpuMetrics snapshot()
}
}
catch (IOException | RuntimeException ex) {
- LOG.error(ex, "Unable to fetch cpu snapshot");
+ LOG.noStackTrace().error(ex, "Unable to fetch cpu snapshot");
Review Comment:
I think the error message can also mention that try using the `CgroupV2CpuMonitor` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org