Skip to content
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

[Tempo] - Overrides section changes #2802

Open
bmgante opened this issue Nov 27, 2023 · 10 comments
Open

[Tempo] - Overrides section changes #2802

bmgante opened this issue Nov 27, 2023 · 10 comments

Comments

@bmgante
Copy link

bmgante commented Nov 27, 2023

Hi,
Wondering to upgrade to tempo 2.3.0 and noticed the following section has changed:
image

The helm chart which is updated seems not to reflect this change, or am i wrong?

  overrides:
    {{- toYaml .Values.global_overrides | nindent 2 }}
    {{- if .Values.metricsGenerator.enabled }}
    metrics_generator_processors:
    {{- range .Values.global_overrides.metrics_generator_processors }}
    - {{ . }}
    {{- end }}
    {{- end }}

My values.yaml needs to be changed? It is currently as below

# Global overrides
global_overrides:
  per_tenant_override_config: /runtime-config/overrides.yaml
  metrics_generator_processors:
    - service-graphs
    - span-metrics

Thanks

@edgarkz
Copy link

edgarkz commented Dec 5, 2023

+1. At this moment the new style is not working using helm value.
As a temporary solution - add manual section:
values: |- overrides: | overrides: "*": ingestion_burst_size_bytes: 50000000 ingestion_rate_limit_bytes: 35000000

@bmgante
Copy link
Author

bmgante commented Dec 5, 2023

What if changing the values yaml this way, would it work?

From:

  overrides:
    {{- toYaml .Values.global_overrides | nindent 2 }}
    {{- if .Values.metricsGenerator.enabled }}
    metrics_generator_processors:
    {{- range .Values.global_overrides.metrics_generator_processors }}
    - {{ . }}
    {{- end }}
    {{- end }}

...
global_overrides:
  per_tenant_override_config: /runtime-config/overrides.yaml
  #metrics_generator_processors: []
  metrics_generator_processors:
    - service-graphs
    - span-metrics

To:

  overrides:
  defaults:
    {{- toYaml .Values.global_overrides | nindent 2 }}
    {{- if .Values.metricsGenerator.enabled }}
    metrics_generator:
    {{- range .Values.global_overrides.metrics_generator.processors }}
    - {{ . }}
    {{- end }}
    {{- end }}

...
global_overrides:
  per_tenant_override_config: /runtime-config/overrides.yaml
  #metrics_generator_processors: []
  metrics_generator:
    processors:
      - service-graphs
      - span-metrics
      - local-blocks

@bmgante
Copy link
Author

bmgante commented Dec 6, 2023

@edgarkz could you please elaborate a bit more on how to apply your manual workaround to have the helm chart working fine with 2.3.x tempo version and the new override syntax?

@AlexDCraig
Copy link
Contributor

AlexDCraig commented Dec 8, 2023

Yeah we definitely need this, looks like #2825 is necessary. If you're like me and have some global overrides it may make sense to wait until this patch is released to upgrade to the chart versions that use 2.3. Losing some of those configs suddenly would have really hurt if I hadn't checked the Grafana update notes. Significant stuff like this should also be in the chart release notes, as this directly relates to the chart

@edgarkz
Copy link

edgarkz commented Dec 13, 2023

@edgarkz could you please elaborate a bit more on how to apply your manual workaround to have the helm chart working fine with 2.3.x tempo version and the new override syntax?

It doesn't work with new syntax.. I have added old syntax overrides to make it work in tempo 2.3
values: |- overrides: | overrides: "*":

@pantuza
Copy link

pantuza commented Jun 11, 2024

TL;DR

On the Helm Chart tempo-distributed instead of adding a parameter overrides, use global_overrides as a workaround:

global_overrides:
  defaults:
    ingestion:
      rate_limit_bytes: 32000000 # 32MB
      burst_size_bytes: 48000000 # 48MB
      max_traces_per_user: 50000

This is a workaround. The global_overrides isn't the right place but since the template outputs its content to the right final overrides block, you can use it by now.

Why the issue is happening?

The Grafana Tempo overrides documentation is correct! If your final yaml file ends up with a overrides block, it will work.
Although, distributed-tempo Helm Chart is where the issue happens. Since version 1.8.0 of this Helm Chart it is using a new block called overrides that accepts a string. Even if you provide the proper string content it will fail simply because the Helm template isn't adding this block on the final yaml file.

Basically, the helm read the content of that block and add to a variable tempo.OverridesConfig (reference).
Then it creates a ConfigMap that uses the content of this variable as a new file overrides.yaml (reference).
And here comes the issue: when it generates the final yaml (tempo.yaml) it does not add the overrides block (reference).
It lets its content on that external file called overrides.yaml.

Possible solutions

I think in this block the Helm Chart should not only add global_overrides but also overrides block as its content.
This would output the overrides content into the final yaml file. Thus, solving the issue because Tempo binary will read the expected overrides block properly.

@batazor
Copy link
Contributor

batazor commented Jul 21, 2024

At the moment I am unable to enable service graph, any tips on how we can do this?

How can I activate enable_virtual_node_label?

Screenshot 2024-07-21 at 11 09 30

@ThomasVitale
Copy link

ThomasVitale commented Jul 29, 2024

The global_overrides doesn't seem to work anymore (chart version 1.15.2). And tempo.structuredConfig can't be used either because of the invalid legacy format used for the multitenant config, even though I'm not using multitenancy. Couldn't find any way to make the metrics generator work with service graph.

@chenlujjj
Copy link

Below is part of my value file which makes the metrics generator work

  # Global overrides
  global_overrides:
    per_tenant_override_config: /runtime-config/overrides.yaml
    defaults:
      metrics_generator:
        processors: [service-graphs, span-metrics]


  # Per tenants overrides
  overrides: {}

chart version is 1.16.2

@ThomasVitale @batazor

@shinebayar-g
Copy link

shinebayar-g commented Nov 7, 2024

Another confused user here. After spending almost an hour reading everywhere, I think the intended usage was:

overrides:
    '*':
        metrics_generator:
            processors: ['service-graphs', 'span-metrics']

Helm chart version: grafana/tempo-distributed: 1.21.0

But as soon as config is applied, I'm getting rate limited errors on the Alloy.

2024-11-07T03:09:59.590772917Z stderr F ts=2024-11-07T03:09:59.590614967Z level=error msg="Exporting failed. Dropping data." component_path=/ component_id=otelcol.exporter.otlp.tempo error="not retryable error: Permanent error: rpc error: code = ResourceExhausted desc = RATE_LIMITED: ingestion rate limit (local: 0 bytes, global: 0 bytes) exceeded while adding 13541 bytes for user single-tenant" dropped_items=10

It seems that override config doesn't merge with the global config, so you have to reconfigure everything again which is crazy and error prone. I don't want to go down this rabbit hole and figure out which configs should be restored in the overrides:.

If my analysis are correct, you should not use overrides: config in the helm chart unless you know what you're doing.
It's not doing overrides as it suggests, it's doing replacements. Opposite of how values.yaml works in helm chart. If I misunderstood the config, please correct me.

Fortunately this config worked for me. Service Graph is populated, Alloy no longer getting rate limited errors.

global_overrides:
    defaults:
        metrics_generator:
            processors: ['service-graphs', 'span-metrics']

@grafana can we improve this situation? overrides: config seems useless as it stands.

Related issues:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants