druid-overlord-extension peonMonitors overwriting problem

[jakubmatyszewski]

Affected Version

26.0.0

Problem

Docs of druid-overlord-extension missing details and/or druid.indexer.runner.peonMonitors not working as expected.

Description

While following instructions of MM-less druid-overlord-extension I had a problem with peons still trying to use monitors defined in Overlord's runtime.properties.

Docs say:

druid.indexer.runner.peonMonitors [JsonArray] - Overrides druid.monitoring.monitors. Use this property if you don't want to inherit monitors from the Overlord

But even though my overlord runtime.properties looked like this:

(...)
druid.monitoring.monitors=["org.apache.druid.java.util.metrics.JvmMonitor", "org.apache.druid.server.metrics.TaskCountStatsMonitor"]

druid.indexer.runner.peonMonitors=["org.apache.druid.java.util.metrics.JvmMonitor", "org.apache.druid.server.metrics.EventReceiverFirehoseMonitor"]

in logs of spawned job pods I received following error:

1) No implementation for org.apache.druid.server.metrics.TaskCountStatsProvider was bound.                                                                                                                 
  while locating org.apache.druid.server.metrics.TaskCountStatsProvider

However on startup of said pods I saw that peonMonitors were registered correctly:

2023-07-17T12:33:56,412 INFO [main] org.apache.druid.cli.CliOverlord - * druid.indexer.runner.peonMonitors: ["org.apache.druid.java.util.metrics.JvmMonitor", "org.apache.druid.server.metrics.EventReceiverFirehoseMonitor"]     

Workaround

Looking into code, I've found that runtime.properties of Overlord are not overridden directly, but rather k8s environment variable called druid_monitoring_monitors is emptied, peons overwrite this variable and it expects to go from here. This is the code.

Workaround for me, just for now is using environment variable in Overlord's runtime.properties like this:

druid.monitoring.monitors=${env:druid_monitoring_monitors:-["org.apache.druid.java.util.metrics.JvmMonitor", "org.apache.druid.server.metrics.TaskCountStatsMonitor"]}

This way I don't have to change overlord's pod spec - it will use default array that is provided, but for peon it will be correctly emptied and applied with array passed to druid.indexer.runner.peonMonitors variable.

Leaving it here in case someone encounters similar issue. I'm not sure if I'd manage to propose a PR to change the code, but perhaps at least I will add a notice about this in docs soon.

[github-actions]

This issue has been marked as stale due to 280 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If this issue is still
relevant, please simply write any comment. Even if closed, you can still revive the
issue at any time or discuss it on the dev@druid.apache.org list.
Thank you for your contributions.

[github-actions]

This issue has been closed due to lack of activity. If you think that
is incorrect, or the issue requires additional review, you can revive the issue at
any time.