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

fix(helm): always create default metrics source if traces are enabled #2182

Merged

Conversation

pmalek-sumo
Copy link
Contributor

@pmalek-sumo pmalek-sumo commented Mar 14, 2022

After introducing: #1647 we got into a bit of a situation for users who enable traces but don't enable metrics.

The problem's root cause is that traces pipeline now requires default metrics endpoint to be available

endpoint: ${SUMO_ENDPOINT_DEFAULT_METRICS_SOURCE}
but doesn't take into account if metrics are enabled in the chart (which in turn controls which sources will be created which in turn controls what will be placed in k8s secret .

This PR addresses that by always creating default metrics endpoint when traces are enabled in the chart.

There's a separate issue that tries to address the meaning of different configuration options in values.yaml that's tracked in #2183.

@pmalek-sumo pmalek-sumo self-assigned this Mar 14, 2022
@pmalek-sumo pmalek-sumo force-pushed the always-create-metrics-source-if-traces-are-enabled branch from 9af6eb6 to f098cd2 Compare March 14, 2022 18:19
@pmalek-sumo pmalek-sumo marked this pull request as ready for review March 14, 2022 18:20
@pmalek-sumo pmalek-sumo requested a review from a team as a code owner March 14, 2022 18:20
*/}}
{{- range $key, $source := $sources }}
{{- if eq $key "default"}}
{{ include "terraform.sources.data" (dict "Endpoint" (include "terraform.sources.config-map-variable" (dict "Type" $type "Context" $ctx "Name" $key)) "Name" (include "terraform.sources.name" (dict "Name" $key "Type" $type))) }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be more readable and less changes if we always create this endpoint (regardless of metrics and tracing configuration). If we have to add it for traces we can add it for logs as well and we just won't use it

Copy link
Contributor

@sumo-drosiek sumo-drosiek left a comment

Choose a reason for hiding this comment

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

Thanks 🚁

@pmalek-sumo pmalek-sumo force-pushed the always-create-metrics-source-if-traces-are-enabled branch from a76f72d to da38548 Compare March 15, 2022 13:44
@github-actions github-actions bot added the documentation documentation label Mar 15, 2022
@pmalek-sumo pmalek-sumo enabled auto-merge (squash) March 15, 2022 13:45
@pmalek-sumo pmalek-sumo merged commit 92ffc6f into main Mar 15, 2022
@pmalek-sumo pmalek-sumo deleted the always-create-metrics-source-if-traces-are-enabled branch March 15, 2022 13:47
@pmalek-sumo pmalek-sumo added this to the v2.6 milestone Mar 15, 2022
@perk-sumo perk-sumo modified the milestones: v2.6, v2.7 Mar 16, 2022
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

4 participants