grafana/k8s-monitoring-helm

Traces and Profiles missing extraConfig block

bentonam opened this issue · 7 comments

bentonam commented

Add support for traces and profiles extraConfig block.

geertn commented

Good to see you are working on this! Is it possible to Cherry pick the fix and merge this already? Currently we are working around this "bug" by patching the configmap after deployment. It would be great if we can leverage your fix without having to patch our helm chart locally.

petewall commented

Currently we have:

extraConfig: ... # Adds config to the "alloy" instance
logs:
  extraConfig: ... # Adds config to the "alloy-logs" instance
  cluster_events:
    extraConfig: ... # Adds config to the "alloy-events" instance
  journal:
    extraConfig: ... # Not used anywhere currently

We could add:

profiles:
  extraConfig: ... # Adds config to the "alloy-profiles" instance

Trace receivers exist on the alloy instance, so the top-level extraConfig should be sufficient.

petewall commented

So, a few options:

  1. Smallest change:
    Add exttraConfig for profiles, the only place that is missing the ability:
profiles:
  extraConfig: ... # Adds config to the "alloy-profiles" instance
  1. More logical sense, but breaking:
    Deprecate the existing extraConfigs and instead put them inside the alloy instances. to make it clearer where they're being deployed:
alloy:
  extraConfig: ...
alloy-events:
  extraConfig: ...
alloy-logs:
  extraConfig: ...
alloy-profiles:
  extraConfig: ...
geertn commented

My thoughts as a user:
Both options would work for me but option 2 seems more logical. Maybe overthinking but since the alloy block seems to already deal with configs

alloy:
  alloy:
    extraConfig: ...
alloy-events:
  alloy:
    extraConfig: ...
alloy-logs:
  alloy:
    extraConfig: ...
alloy-profiles:
  alloy:
    extraConfig: ...

Afterburner: is there a reason alloy isn't named alloy-metrics?

petewall commented

Let me chat with some folks internally and we'll make a plan.

Afterburner: is there a reason alloy isn't named alloy-metrics?

Because the "main" alloy instance scrapes metrics, but also opens the receivers, which then can pick up traces, as well as metrics and logs pushed via OTLP, OTLP HTTP, Zipkin, Jaeger, etc... It mostly does metrics, but it can do much more than just metrics.

  • 👍1
geertn commented

I assume it's easiest and least intrusive to just merge adding the extraConfig block to profiles:. If this can be merged soonish it would help us. We're in the process of implementing Continuous Profiling and I can't really move forward to production with needing a workaround (patching ConfigMap after Helm run).

geertn commented

I now see that you merged another way to configure Profiles. I've got my usecase to work with the helm values below. Please disregard my comment above, thank you!

profiles:
  enabled: true
  java:
    enabled: true
    namespaces: [${PYRONS}]
    extraRelabelingRules: |-
      rule {
        source_labels = ["__meta_kubernetes_pod_annotation_profiling_disabled"]
        regex = "(true)"
        action = "drop"
      }
    profilingConfig: {"alloc":"512k","cpu":true,"interval":"30s","lock":"","sampleRate":97}
  ebpf:
    enabled: false
  • ❤️1