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

Add metricsSourceType tracesSourceType #1376

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

mstopa-splunk
Copy link

Description: Added metricsSourceType and tracesSourceType to the chart
Link to Splunk idea:
Testing: manual
Documentation: N/A

@mstopa-splunk mstopa-splunk requested review from a team as code owners August 8, 2024 13:55
@mstopa-splunk mstopa-splunk changed the title Add metrics source type traces source type Add metricsSourceType tracesSourceType Aug 8, 2024
helm-charts/splunk-otel-collector/values.schema.json Outdated Show resolved Hide resolved
@@ -391,6 +391,9 @@ splunk_hec/platform_metrics:
token: "${SPLUNK_PLATFORM_HEC_TOKEN}"
index: {{ .Values.splunkPlatform.metricsIndex | quote }}
source: {{ .Values.splunkPlatform.source | quote }}
{{- if .Values.splunkPlatform.metricsSourceType }}
sourcetype: {{ .Values.splunkPlatform.metricsSourceType | quote }}
{{- end }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like it may get confusing since we are adding a sourcetype to logs with "resource/logs" processor. Maybe we could create something similar for metrics/traces?

Copy link
Author

Choose a reason for hiding this comment

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

@sbylica-splunk I can work on that.
On the other hand we set all other Splunk metadata for logs/metrics/traces in the hec exporters, so it's also quite an intuitive place for setting the sourcetype as well.
What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, that makes more sense in my opinion. Do mind though that this would add a sourcetype everywhere - to the gateway and receiver too. If that's okay then I think we can move it to hec_exporters

Copy link
Contributor

@jvoravong jvoravong Sep 20, 2024

Choose a reason for hiding this comment

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

I think there are still some gaps here for telemetry data support for a splunk-platform-enable-all-telemetry values configuration.

  • Can you create a example in the /examples dir so we can see the for differences this PR would add? I think it would be worth having an example where all telemetry types are enabled for splunkPlatform.*. This will likely make reviews easier as well.
  • You can also even add unit tests under https://github.com/signalfx/splunk-otel-collector-chart/tree/main/test/unittests to help ensure added Helm template logic.

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.

3 participants