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

feat: add metadata.metrics.config.extraProcessors #2724

Merged
merged 8 commits into from
Jan 3, 2023

Conversation

sumo-drosiek
Copy link
Contributor

Signed-off-by: Dominik Rosiek drosiek@sumologic.com

Description

Fill in your description here.


Checklist
  • Changelog updated
Testing performed
  • Redeploy fluentd and fluentd-events pods
  • Confirm events, logs, and metrics are coming in

@github-actions github-actions bot added the documentation documentation label Dec 22, 2022
@sumo-drosiek sumo-drosiek marked this pull request as ready for review December 22, 2022 14:00
@sumo-drosiek sumo-drosiek requested a review from a team as a code owner December 22, 2022 14:00
require.Equal(t, renameMetadatatatements, otelConfig.Processors.RenameMetadata.MetricStatements[0].Statements)
}

func TestMetadataMetricsOtelConfigExtraProcessorsPipeline(t *testing.T) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two tests use the same configuration, could we merge them into one? The actual assertions in this test aren't very complex, adding them to the first test should be ok.

I'd also consider moving the valuesYaml to a file in testdata/, it's a bit large for an inline value imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

deploy/helm/sumologic/values.yaml Show resolved Hide resolved
docs/collecting-application-metrics.md Show resolved Hide resolved
@@ -209,3 +220,127 @@ To scrape and forward exposed metrics to Sumo Logic, please follow one of the fo

- [Application metrics are exposed (one endpoint scenario)](#application-metrics-are-exposed-one-endpoint-scenario)
- [Application metrics are exposed (multiple enpoints scenario)](#application-metrics-are-exposed-multiple-enpoints-scenario)

## Metrics modifications
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also update https://github.com/SumoLogic/sumologic-kubernetes-collection/blob/main/docs/best-practices.md#excluding-metrics and other metrics sections in the best practices doc with this new info?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I write more information there or linking to this document will be sufficient?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd provide some examples. It's structurally not the same thing either, as you may want to filter and modify non-application metrics.

Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants