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

Enable batch for metrics #90

Merged
merged 3 commits into from
Apr 13, 2021
Merged

Enable batch for metrics #90

merged 3 commits into from
Apr 13, 2021

Conversation

jrcamp
Copy link
Contributor

@jrcamp jrcamp commented Apr 12, 2021

Use default config for batch (send every 200ms or 8192 items). This is same
that
https://github.com/signalfx/splunk-otel-collector/blob/main/cmd/otelcol/config/collector/gateway_config.yaml
is using.

Resolves #32

Use default config for batch (send every 200ms or 8192 items). This is same
that
https://github.com/signalfx/splunk-otel-collector/blob/main/cmd/otelcol/config/collector/gateway_config.yaml
is using.

Resolves signalfx#32
@jrcamp jrcamp requested review from dmitryax and mstumpfx April 12, 2021 17:13
@@ -111,8 +111,6 @@ processors:
{{- include "splunk-otel-collector.otelMemoryLimiterConfig" .Values.otelAgent | nindent 2 }}

batch:
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember facing issues with null values in helm. Can we set batch: {} instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIR you cannot override them in user's values files. Feel free to try, I may be wrong

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems to work fine merging in values from otelAgent.processors.batch. We do the same thing with zpages extension that was already there so I think you're thinking of a different issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you might be thinking of a situation where in values.yaml we had to do foo: {} instead of foo: due to some reason.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, maybe. I don't remember exact reason

@jrcamp jrcamp merged commit 3a045a7 into signalfx:main Apr 13, 2021
@jrcamp jrcamp deleted the batch branch April 13, 2021 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable batching for metrics
2 participants