-
Notifications
You must be signed in to change notification settings - Fork 17
Splunk specific file based configuration fields #355
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
Changes from 2 commits
d437cda
1123de3
f80d5aa
1bff279
a6670b0
ea159c0
6bb05d8
69a19c8
fda8404
6a0e2c8
86e1dc9
1251ba2
0b64f05
45a27be
0310b22
5af67f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -239,8 +239,8 @@ | |
| a deprecation warning and suggest an alternate method. For example: | ||
| ```txt | ||
| jaeger-thrift-splunk trace exporter is deprecated and may be removed in a future major release. Use the default | ||
| OTLP exporter instead, or set the SPLUNK_REALM and SPLUNK_ACCESS_TOKEN environment variables to send | ||
| jaeger-thrift-splunk trace exporter is deprecated and may be removed in a future major release. Use the default | ||
| OTLP exporter instead, or set the SPLUNK_REALM and SPLUNK_ACCESS_TOKEN environment variables to send | ||
| telemetry directly to Splunk Observability Cloud. | ||
| ``` | ||
|
|
@@ -250,12 +250,73 @@ | |
| In addition to environment variables, other ways of defining configuration also exist: | ||
| - [Java System | ||
| Properties](https://docs.oracle.com/javase/tutorial/essential/environment/sysprop.html): | ||
| These properties MUST match the environment variables converting to lower | ||
| case and replacing underscores with hyphens or periods. For example: | ||
| system property `splunk.trace-response-header.enabled` is equivalent to environment | ||
| variable `SPLUNK_TRACE_RESPONSE_HEADER_ENABLED`. | ||
| #### File based configuration | ||
|
Check failure on line 253 in specification/configuration.md
|
||
| OpenTelemetry's [declarative configuration](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/README.md#declarative-configuration) SHOULD be supported via [`OTEL_EXPERIMENTAL_CONFIG_FILE`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk.md#via-otel_experimental_config_file) environment variable. | ||
|
Check failure on line 255 in specification/configuration.md
|
||
| In addition, Splunk specific configuration MUST have their own root-level configuration block named `splunk`. | ||
|
Check failure on line 257 in specification/configuration.md
|
||
| The following is an example covering the common configuration values: | ||
| ```yaml | ||
| distribution: | ||
| splunk: | ||
| tracing: | ||
| disabled: false | ||
| response_header_enabled: true # SPLUNK_TRACE_RESPONSE_HEADER_ENABLED | ||
| profiling: | ||
| disabled: false # SPLUNK_PROFILER_ENABLED | ||
| endpoint: "" # SPLUNK_PROFILER_LOGS_ENDPOINT | ||
| memory_profiling_enabled: true # SPLUNK_PROFILER_MEMORY_ENABLED | ||
| callgraphs: | ||
| disabled: false # SPLUNK_SNAPSHOT_PROFILER_ENABLED | ||
| sampling_interval: 10 # SPLUNK_SNAPSHOT_SAMPLING_INTERVAL | ||
| selection_probability: 0.01 # SPLUNK_SNAPSHOT_SAMPLING_INTERVAL | ||
| # Language specific configurations | ||
| runtime: | ||
| nodejs: | ||
| metrics: | ||
| runtime_metrics: | ||
| enabled: true # SPLUNK_RUNTIME_METRICS_ENABLED | ||
| collection_interval: 30000 # SPLUNK_RUNTIME_METRICS_COLLECTION_INTERVAL | ||
| instrumentation_metrics_enabled: true # SPLUNK_INSTRUMENTATION_METRICS_ENABLED | ||
| autoinstrument_packages: | ||
| - "MyApiService" | ||
| - "MyOtherService" | ||
| java: | ||
| ... | ||
|
||
| tracer_provider: | ||
| processors: | ||
| - batch: | ||
| exporter: | ||
| otlp_http: | ||
| endpoint: "" | ||
| headers: | ||
| - name: "X-SF-TOKEN" | ||
| - value: "" | ||
| meter_provider: | ||
| readers: | ||
| - periodic: | ||
| exporter: | ||
| otlp_http: | ||
| endpoint: "" | ||
| headers: | ||
| - name: "X-SF-TOKEN" | ||
| - value: "" | ||
Kielek marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| logger_provider: | ||
| processors: | ||
| - batch: | ||
| exporter: | ||
| otlp_http: | ||
| endpoint: "" | ||
| ``` | ||
|
|
||
| #### [Java SystemProperties](https://docs.oracle.com/javase/tutorial/essential/environment/sysprop.html) | ||
|
|
||
| These properties MUST match the environment variables converting to lower | ||
Kielek marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| case and replacing underscores with hyphens or periods. For example: | ||
| system property `splunk.trace-response-header.enabled` is equivalent to environment | ||
| variable `SPLUNK_TRACE_RESPONSE_HEADER_ENABLED`. | ||
|
|
||
| ### Snapshot Profiler | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be something like
Discussed with @lachmatt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This (memory_allocation_sampling, callstack_sampling, snapshotting) is completely different wording than we have in our documentation. Why would you not keep parity here?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Together with @lachmatt we find the current wording (from the documentation) confusing and misleading. We propose changing the names to better represent the behavior of the profilers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the proposal misses some of the configuration supported by Java.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Giving it some thought again after our discussion, I think it doesn't make sense because of 2 reasons:
An unrelated to your comment, but a related issue is that while going over the configuration, I noticed that you can't actually set the endpoint for callgraphs explicitly with file based config. The documentation we have refers to
OTEL_EXPORTER_OTLP_ENDPOINT. However this does not make sense much with file based config when the pipelines are in their own sections.We either have to create a new configuration option for a callgraphs endpoint or we use the one from profiler.
Now both the CPU profiler and callgraphs can be active irrespective of each other. When (as previously proposed) we don't have an
enabled/disabledflag under profiling we can end up in a case where we can't set the endpoint for callgraphs, e.g.:Some of the options we have:
or
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And my question about
otlp_log_http.I think we should use the term from the spec,
otlp_http/otlp_grpc.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that we are using a "log OTLP exporter" not "profiling OTLP expoter" which we may want to in future. This is why I added
_log_😉There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey is there any significant reason for changing the naming of profiling related attributes?
I would preferer to keep it fairly simple and using terms which are more common in the industry (cpu/memory profiler / profiling etc)
Also using term "snapshotting" sounds bit ambiguous - as snapshot can be almost a specific time-bounded data of any kind where callgraphs or stack traces more specifically call it what it is
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akubik-splunk note we are already using term "snapshots" in e.g. env variables in the spec