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

Metrics index from annotations #1053

Merged
merged 9 commits into from
Jan 16, 2024

Conversation

sbylica-splunk
Copy link
Collaborator

@sbylica-splunk sbylica-splunk commented Dec 1, 2023

Description:
Added support for annotations in metrics index

Testing:
Unit test

Documentation:
Added short explanation to advanced-configuration.md

@sbylica-splunk sbylica-splunk requested review from a team as code owners December 1, 2023 10:27
@rmfitzpatrick
Copy link
Contributor

@sbylica-splunk why is a new k8sattributes processor being added for all components instead of enhancing the existing agent's? I don't believe these attrs are necessary for o11y cloud cases so limiting to platform w/ metrics configurations seems preferable as well.

@atoulme
Copy link
Contributor

atoulme commented Dec 1, 2023

Please rebase and rerun make render

@sbylica-splunk
Copy link
Collaborator Author

Please rebase and rerun make render

Done

@sbylica-splunk
Copy link
Collaborator Author

sbylica-splunk commented Dec 4, 2023

o11y cloud cases

I added a new processor because we already have similar use case for logs (k8sattributes processor) and I wanted to make sure it works the same way.
As for you second question, do you mean something like that?

  {{- if (eq (include "splunk-otel-collector.platformMetricsEnabled" $) "true") }}
  -  k8sattributes/metrics
  {{- end }}

@rmfitzpatrick
Copy link
Contributor

As for you second question, do you mean something like that?

Yes, though I think we'll also want to ensure that the new attributes aren't exported as o11y metric dimensions via the signalfx exporter. I'm not sure if there's a way to do that other than duplicating pipelines or using a connector w/ an additional transform processor. Maybe @dmitryax has suggestions since I think he's better versed on the use case than I am.

@dmitryax
Copy link
Contributor

dmitryax commented Dec 5, 2023

@sbylica-splunk please make it conditional. At least if splunk-otel-collector.platformMetricsEnabled as @rmfitzpatrick suggested. Also, keep in mind that it introduces a significant memory utilization overhead

@sbylica-splunk sbylica-splunk force-pushed the metrics_index_from_annotations branch 2 times, most recently from e41809e to fa754f7 Compare December 13, 2023 12:47
@sbylica-splunk
Copy link
Collaborator Author

Ok, so is there anything else to do? I'll squash current commits after this pr is good to go :)

@jinja2
Copy link
Collaborator

jinja2 commented Dec 14, 2023

@sbylica-splunk please fix trailing whitespace issues as seen in the failing pre-commit job

@dmitryax dmitryax merged commit ba94bf3 into signalfx:main Jan 16, 2024
32 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants