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

Allow Exporting of the Collector's internal spans #6629

Open
TylerHelmuth opened this issue Nov 28, 2022 · 15 comments
Open

Allow Exporting of the Collector's internal spans #6629

TylerHelmuth opened this issue Nov 28, 2022 · 15 comments

Comments

@TylerHelmuth
Copy link
Member

TylerHelmuth commented Nov 28, 2022

Is your feature request related to a problem? Please describe.
Currently the collector can be made to emit metric and log telemetry, but not spans. The collector does have the capability to generate its own spans, but they can only be viewed with the zpages extension. It would be best if the spans could also be emitted like the collector's metrics and logs

Describe the solution you'd like
The collector's telemetry settings allow for configuring a "output" for spans that result in the spans being sent through a collector pipeline.

I think it would be best if the data could be sent to a pipeline directly without needing to go over the network. If that isn't possible then maybe we restrict exporting of spans to the OTLP format and require a OTLP receiver to collect the collector's spans?

The configuration could look something like:

// TracesConfig exposes the common Telemetry configuration for collector's internal spans.
// Experimental: *NOTE* this structure is subject to change or removal in the future.
type TracesConfig struct {
	// Propagators is a list of TextMapPropagators from the supported propagators list. Currently,
	// tracecontext and  b3 are supported. By default, the value is set to empty list and
	// context propagation is disabled.
	Propagators []string `mapstructure:"propagators"`

	// Outputs is a list of pipeline names that should receive the collector's spans.
	// If no outputs are specified then spans are not exported from the collector.
	Outputs []string `mapstructure:"outputs"`
}
service:
  telemetry:
    traces:
      outputs:
        - "pipeline"

Maybe we could use the default pipeline if no outputs are specific, essentially making span exporting the default behavior, and include some other config option to disable span exporting?

@TylerHelmuth TylerHelmuth changed the title Allow Exporting of the Collector's internal tracing Allow Exporting of the Collector's internal spans Nov 28, 2022
@jpkrohling
Copy link
Member

If the OTLP receiver and exporter are instrumented with tracing and exporting is the default behavior, wouldn't it just cause an endless stream of spans by default?

@TylerHelmuth
Copy link
Member Author

@jpkrohling maybe the solution is being able to define in the telemetry settings a pipeline specific to the collector's internal telemetry and this pipeline doesn't instrument itself (somehow)? Could look like:

service:
  telemetry:
    pipeline:
      traces:
        processors: []
        exporters: []
      metrics:
        processors: []
        exporters: []
      logs:
        processors: []
        exporters: []

@jpkrohling
Copy link
Member

I have a bad feeling about this one. The common practice I've been seeing is to have the monitors being external to the process being monitored, and I think this applies here as well ("who monitors the monitor" type of question). In that sense, I would recommend our users use an external OTLP endpoint whenever possible to receive the collector's telemetry data and would not have this enabled by default.

@TylerHelmuth
Copy link
Member Author

TylerHelmuth commented Nov 29, 2022

@jpkrohling an external endpoint is ok as well, although we lose potential processing power. As long as we can get the spans out I'll be happy.

@TylerHelmuth
Copy link
Member Author

Based on the SIG discussion today I will move forward with a config proposal to allow configuring exporters in the OpenTelemetry Go SDK.

@TylerHelmuth
Copy link
Member Author

TylerHelmuth commented Nov 30, 2022

@bogdandrutu @jpkrohling @codeboten @Aneurysm9 was the idea of using ENV VARs to configure the exporter(s) completely rejected? If they are allowed, which takes precedence, env vars or the explicit configuration in TelemetrySettings?

@bogdandrutu
Copy link
Member

I think we should not support that for the moment, idea being that these kind of decisions should be made by the config SIG and by not supporting I believe that we are postponing that decision.

@TylerHelmuth
Copy link
Member Author

TylerHelmuth commented Dec 1, 2022

Current Requirements:

  1. The configuration must be generic enough to apply to any type of exporter (even if we only support OTLP at the start)
  2. Should be able to be extended to include metrics and logs in the future.
  3. ENV VARs should not be used to do the configuring (for now).

SDK Exporter Configuration

  • The Go SDK exporters all use the Go pattern of Options to configure the exporter.
  • These Options vary by exporter type.
  • Each option can have different parameters

To be able to configure the SDK we will need to be able to identify which options should be used and the parameters

Collector TelemetrySettings Configuration Proposal

I think we can handle mapping from configuration to Option via a combination of the exporter’s name and the option name.

service:
  telemetry:
    traces:
      exporters:
        - name: "exporter name"
          options: 
            - name: "option name"
              parameters: 
                - "string"
                - "string"
            - name: "option name"
              parameters: 
                - "string"
                - "string"
        - name: "exporter name"
          options: 
            - name: "option name"
              parameters: 
                - "string"
                - "string"
            - name: "option name"
              parameters: 
                - "string"
                - "string"

Downsides:

  • User must know the Go SDK in order to configure the collector’s use of the Go SDK. If the user doesn’t know what exporters/options are available (and no using Intelisense here to help) it will be difficult to configure.
  • Maybe doesn’t extend to other languages, won’t fit in with the Configuration SIG’s goals?

Upsides:

  • Generic enough to handle any type of exporter, as long as it uses options.
  • Explicit; all options are available and configurable.
  • Scalable; as more options/exporters are added the configuration doesn’t have to change

@jpkrohling
Copy link
Member

Are we allowing users to configure other settings, like samplers, tracer names, and custom resource attributes? I think it would also be good to have a property to enable tracing, like:

service:
  telemetry:
    traces:
      enabled: true

@TylerHelmuth
Copy link
Member Author

TylerHelmuth commented Dec 5, 2022

@jpkrohling we don't currently have an enable switch for metrics or logs or the traces that are created today. If we add one for tracing then I think it should be consistent and we should add some for metric and log telemetry as well. The lack of any exporter configuration could also be used to signify spans should not be exported but should continue to be created and exposed via zpages.

I think we will need to be able to configure samplers and tracer name (although I think we should supply defaults for both). Custom resource attributes should reuse the telemetry setting's existing Resource field I think.

@codeboten
Copy link
Contributor

@TylerHelmuth adding the current example configuration from the configuration working group since we're essentially configuring the OTel Go SDK, I would lean towards supporting the same configuration:

  tracer_provider:
    # Span exporters. Each exporter key refers to the type of the exporter. Values configure the exporter. Exporters must be associated with a span processor.
    exporters:
      # Configure the zipkin exporter.
      zipkin:
        # Sets the endpoint.
        #
        # Environment variable: OTEL_EXPORTER_ZIPKIN_ENDPOINT
        endpoint: http://localhost:9411/api/v2/spans
        # Sets the max time to wait for each export.
        #
        # Environment variable: OTEL_EXPORTER_ZIPKIN_TIMEOUT
        timeout: 10000
    # List of span processors. Each span processor has a name and args used to configure it.
    span_processors:
      # Add a batch span processor.
      #
      # Environment variable: OTEL_BSP_*, OTEL_TRACES_EXPORTER
      - name: batch
        # Configure the batch span processor.
        args:
          # Sets the delay interval between two consecutive exports.
          #
          # Environment variable: OTEL_BSP_SCHEDULE_DELAY
          schedule_delay: 5000
          # Sets the maximum allowed time to export data.
          #
          # Environment variable: OTEL_BSP_EXPORT_TIMEOUT
          export_timeout: 30000
          # Sets the maximum queue size.
          #
          # Environment variable: OTEL_BSP_MAX_QUEUE_SIZE
          max_queue_size: 2048
          # Sets the maximum batch size.
          #
          # Environment variable: OTEL_BSP_MAX_EXPORT_BATCH_SIZE
          max_export_batch_size: 512
          # Sets the exporter. Exporter must refer to a key in sdk.tracer_provider.exporters.
          #
          # Environment variable: OTEL_TRACES_EXPORTER
          exporter: zipkin

See https://github.com/MrAlias/otel-schema/blob/332987fc1980b2631122771c8d721fa3e7ab32d4/config.yaml#L35-L77 for additional configuration

@TylerHelmuth
Copy link
Member Author

@codeboten I'm all for using what the Working Group is creating. 1 question, is the trace_provider.exporters section a list or a map? Would the collector's implementation need to update any time we need to support a new exporter?

@codeboten
Copy link
Contributor

@codeboten I'm all for using what the Working Group is creating.

💯, otherwise i imagine this would be confusing for users. Although tbh, there are a lot of ways this exporter configuration could be confusing to users. I'm already asking myself the question why is this configuration different than the other exporters 🥲

1 question, is the trace_provider.exporters section a list or a map?

The exporters section is currently a map. I think there was talk of supporting a similar pattern as the collector's zipkin/somewhere zipkin/somewhereelse to identify multiple instances of the same exporter with different configuration.

Would the collector's implementation need to update any time we need to support a new exporter?

I would expect this to be handled by the otel go sdk, so if the otel go sdk supports a new exporter, then the collector would need to update the dependency.

@TylerHelmuth
Copy link
Member Author

@codeboten with that configuration which fields does the working group think should be required? Does the working group expect certain fields to have default values?

Default values may be tricky for us. At the moment we are using the presence of tracer_provider to mean that the traces should be exported. I think it will be hard to detect when the user has specified tracer_provider but not all the fields and then default the un-supplied fields.

As an inital go at this feature it may be easiest to require the user to explicitly specify all config fields they need.

@atoulme
Copy link
Contributor

atoulme commented Dec 19, 2023

It looks to me that open-telemetry/opentelemetry-collector-contrib#28994 indicates this issue is now resolved. Anything left to do here?

codeboten added a commit that referenced this issue Sep 9, 2024
…o beta (#11091)

#### Description

This configuration of the internal collector telemetry has been using
the SDK configuration for some time. Moving this gate from alpha to
beta.

#### Link to tracking issue
Related to #6629

---------

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
dmathieu pushed a commit to dmathieu/opentelemetry-collector that referenced this issue Sep 10, 2024
…o beta (open-telemetry#11091)

#### Description

This configuration of the internal collector telemetry has been using
the SDK configuration for some time. Moving this gate from alpha to
beta.

#### Link to tracking issue
Related to open-telemetry#6629

---------

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
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

No branches or pull requests

5 participants