Open MarcinGinszt opened 1 year ago
This issue has been automatically marked as stale because it has not had any activity in the past 60 days. The next time this stale check runs, the stale label will be removed if there is new activity. The issue will be closed after 15 days if there is no new activity. Please apply keepalive label to exempt this Issue.
I am experiencing this with 2.6.0.
The following config does not properly prefix labels:
metrics_generator:
processor:
service_graphs:
dimensions:
- foo
enable_client_server_prefix: true
Adding the following (as suggested in https://github.com/utilitywarehouse/opentelemetry-manifests/pull/15) works around it:
overrides:
metrics_generator_processor_service_graphs_enable_client_server_prefix: true
I think this code is causing the issue:
When we initialize the service graph processor, we copy the setting from the overrides without checking if a value is already set or not.
But... because this field is a bool
, it's actually impossible to tell if it has been set or not. It can either be false
or true
, in which false
could mean "I've set this to false" or "I have not set this, it defaulted to false".
So unfortunately this configuration field has actually become useless after we added the override for it.
The value will always be replaced by the value in the overrides. Meaning if you don't set overrides for your tenant it will be reset to false
.
It's a known shortcoming that our overrides don't allow leaving a field unset. Empty fields will always get the default value for their type, which for boolean values is false
. For string and numbers this is less of an issue.
In the user-configurable overrides we have addressed this by using *bool
instead. So if we ever refactor the runtime overrides entirely we could maybe fix this too.
For clarity I'd suggest removing the configuration option from regular config block and only allow it to be set through overrides.
Describe the bug Metrics-generator
enable_client_server_prefix
doesn't actually enable the client server prefix without also settingmetrics_generator_processor_service_graphs_enable_client_server_prefix
to true.[Our configuration with](https://github.com/utilitywarehouse/opentelemetry-manifests/blob/main/tempo/base/config.yaml#L29)
enable_client_server_prefix
and without tenant override doesn't add prefixes to metric labels.Request to
/status/config
confirms, that theenable_client_server_prefix
is set totrue
. However, it also reports thatmetrics_generator_processor_service_graphs_enable_client_server_prefix
is set tofalse
, even though it is not defined in our configuration.Setting in our config
metrics_generator_processor_service_graphs_enable_client_server_prefix
totrue
resolves the issue- prefixes are correctly added.We are using Tempo version 2.2.0, which should have this feature enabled. To Reproduce
enable_client_server_prefix
in metric generator config to true, while not setttingmetrics_generator_processor_service_graphs_enable_client_server_prefix
.metrics_generator_processor_service_graphs_enable_client_server_prefix
to your configurationExpected behavior Overrides should not work until they are defined
Environment: