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

Traces and Profiles missing extraConfig block #565

Open
bentonam opened this issue Jun 10, 2024 · 7 comments · May be fixed by #646
Open

Traces and Profiles missing extraConfig block #565

bentonam opened this issue Jun 10, 2024 · 7 comments · May be fixed by #646
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@bentonam
Copy link
Collaborator

Add support for traces and profiles extraConfig block.

@bentonam bentonam added the enhancement New feature or request label Jun 10, 2024
@bentonam bentonam self-assigned this Jun 10, 2024
bentonam added a commit that referenced this issue Jun 10, 2024
bentonam added a commit that referenced this issue Jun 10, 2024
@geertn
Copy link

geertn commented Jun 27, 2024

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
Copy link
Collaborator

petewall commented Jun 27, 2024

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
Copy link
Collaborator

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
Copy link

geertn commented Jun 27, 2024

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
Copy link
Collaborator

petewall commented Jun 27, 2024

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.

petewall pushed a commit that referenced this issue Jul 5, 2024
petewall pushed a commit that referenced this issue Jul 5, 2024
@geertn
Copy link

geertn commented Jul 16, 2024

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
Copy link

geertn commented Jul 16, 2024

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

petewall pushed a commit that referenced this issue Jul 16, 2024
petewall pushed a commit that referenced this issue Jul 16, 2024
@petewall petewall linked a pull request Jul 19, 2024 that will close this issue
@petewall petewall added this to the 2.0 milestone Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants