Panic on collector start when using otlp exporter with tls: insecure: true and headers_setter extension

[gburek-fastly]

Component(s)

extension/headerssetter

What happened?

Description

When using the headers_setter extension with an otlp exporter, configured with tls: insecure: true, the contrib collector panics on start. Removing the tls: insecure: true or the headers_setter extension from the exporter config, allows start up to succeed.

Steps to Reproduce

Using the attached config.yaml, run:

docker run -v $(pwd)/config.yaml:/etc/otelcol-contrib/config.yaml otel/opentelemetry-collector-contrib:0.66.0

Expected Result

The collector starts and uses context to attach a header to all traces exported

Actual Result

The collector crashes and does not start

Collector version

v0.66.0

Environment information

Environment

OS: OSX 12.6.1
Docker Desktop 4.13.1

OpenTelemetry Collector configuration

extensions:
  headers_setter:
    headers:
      - key: X-Scope-OrgID
        from_context: tenant_id
      - key: User-ID
        value: user_id

receivers:
  otlp:
    protocols:
      http:
        include_metadata: true

exporters:
  otlp:
    endpoint: otelcol:4317
    tls:
      insecure: true
    auth:
      authenticator: headers_setter

service:
  extensions: [ headers_setter ]
  pipelines:
    traces:
      receivers: [ otlp ]
      exporters: [ otlp ]

Log output

» docker run -v $(pwd)/config.yaml:/etc/otelcol-contrib/config.yaml otel/opentelemetry-collector-contrib:0.66.0
2022/11/28 23:06:23 proto: duplicate proto type registered: jaeger.api_v2.PostSpansRequest
2022/11/28 23:06:23 proto: duplicate proto type registered: jaeger.api_v2.PostSpansResponse
2022-11-28T23:06:23.263Z        info    service/telemetry.go:111        Setting up own telemetry...
2022-11-28T23:06:23.263Z        info    service/telemetry.go:141        Serving Prometheus metrics      {"address": ":8888", "level": "Basic"}
2022-11-28T23:06:23.264Z        info    service/service.go:89   Starting otelcol-contrib...     {"Version": "0.66.0", "NumCPU": 6}
2022-11-28T23:06:23.264Z        info    extensions/extensions.go:41     Starting extensions...
2022-11-28T23:06:23.264Z        info    extensions/extensions.go:44     Extension is starting...        {"kind": "extension", "name": "headers_setter"}
2022-11-28T23:06:23.264Z        info    extensions/extensions.go:48     Extension started.      {"kind": "extension", "name": "headers_setter"}
2022-11-28T23:06:23.264Z        info    pipelines/pipelines.go:74       Starting exporters...
2022-11-28T23:06:23.264Z        info    pipelines/pipelines.go:78       Exporter is starting... {"kind": "exporter", "data_type": "traces", "name": "otlp"}
2022-11-28T23:06:23.264Z        info    service/service.go:115  Starting shutdown...
2022-11-28T23:06:23.264Z        info    pipelines/pipelines.go:118      Stopping receivers...
2022-11-28T23:06:23.264Z        info    pipelines/pipelines.go:125      Stopping processors...
2022-11-28T23:06:23.264Z        info    pipelines/pipelines.go:132      Stopping exporters...
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x550fa0]

goroutine 1 [running]:
google.golang.org/grpc.(*ClientConn).Close(0x0)
        google.golang.org/[email protected]/clientconn.go:1016 +0x30
go.opentelemetry.io/collector/exporter/otlpexporter.(*exporter).shutdown(0xffffffffffffffff?, {0x5f09247?, 0x4?})
        go.opentelemetry.io/collector/exporter/[email protected]/otlp.go:93 +0x20
go.opentelemetry.io/collector/component.ShutdownFunc.Shutdown(...)
        go.opentelemetry.io/collector/[email protected]/component.go:95
go.opentelemetry.io/collector/exporter/exporterhelper.newBaseExporter.func2({0x6bd7500, 0x40000b6018})
        go.opentelemetry.io/[email protected]/exporter/exporterhelper/common.go:182 +0x60
go.opentelemetry.io/collector/component.ShutdownFunc.Shutdown(...)
        go.opentelemetry.io/collector/[email protected]/component.go:95
go.opentelemetry.io/collector/service/internal/pipelines.(*Pipelines).ShutdownAll(0x4000b92be0, {0x6bd7500, 0x40000b6018})
        go.opentelemetry.io/[email protected]/service/internal/pipelines/pipelines.go:135 +0x294
go.opentelemetry.io/collector/service.(*service).Shutdown(0x4000b7d080, {0x6bd7500, 0x40000b6018})
        go.opentelemetry.io/[email protected]/service/service.go:121 +0xd0
go.opentelemetry.io/collector/service.(*Collector).shutdownServiceAndTelemetry(0x40010eba70, {0x6bd7500?, 0x40000b6018?})
        go.opentelemetry.io/[email protected]/service/collector.go:264 +0x30
go.opentelemetry.io/collector/service.(*Collector).setupConfigurationComponents(0x40010eba70, {0x6bd7500, 0x40000b6018})
        go.opentelemetry.io/[email protected]/service/collector.go:166 +0x1fc
go.opentelemetry.io/collector/service.(*Collector).Run(0x40010eba70, {0x6bd7500, 0x40000b6018})
        go.opentelemetry.io/[email protected]/service/collector.go:190 +0x30
go.opentelemetry.io/collector/service.NewCommand.func1(0x400056f200, {0x5f1280e?, 0x2?, 0x2?})
        go.opentelemetry.io/[email protected]/service/command.go:53 +0x3b8
github.com/spf13/cobra.(*Command).execute(0x400056f200, {0x40000b0220, 0x2, 0x2})
        github.com/spf13/[email protected]/command.go:916 +0x5e0
github.com/spf13/cobra.(*Command).ExecuteC(0x400056f200)
        github.com/spf13/[email protected]/command.go:1044 +0x368
github.com/spf13/cobra.(*Command).Execute(...)
        github.com/spf13/[email protected]/command.go:968
main.runInteractive({{0x40009fc360, 0x40009fd530, 0x40009fc780, 0x40009fc000}, {{0x5f3b360, 0xf}, {0x5fb82c8, 0x1f}, {0x5f0c283, 0x6}}, ...})
        github.com/open-telemetry/opentelemetry-collector-releases/contrib/main.go:32 +0x40
main.run(...)
        github.com/open-telemetry/opentelemetry-collector-releases/contrib/main_others.go:11
main.main()
        github.com/open-telemetry/opentelemetry-collector-releases/contrib/main.go:25 +0x124

Additional context

Context is that I am attempting to use the headers_setter extension to add a multi-tenancy header to the otlp grpc exporter pointing at a tempo backend, as mentioned in https://grafana.com/docs/tempo/latest/configuration/multitenancy/

[github-actions]

Pinging code owners:

• extension/headerssetter: @jpkrohling @kovrus

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[pavankrish123]

This is related (duplicate) to open-telemetry/opentelemetry-collector#6619. Work in progress. I will try to get the fix in as soon as I can. But please note that even after the fix, the collector will still refuse to communicate insecurely.

[gburek-fastly]

@pavankrish123 That seems right, but a pain to require TLS for dynamic headers.

Further background is that we use a linkerd service mesh to provide mTLS connections between kubernetes components, so the requirement of adding TLS inside the service mesh TLS, to use an auth extension seems extremely strict.

[pavankrish123]

@gburek-fastly, I think the behavior is expected from underlying grpc framework if we want to use any credentials.

As I did mention above the fix I am working on will alleviate the panic but the collector still fails gracefully.

Adding @jpkrohling for additional suggestions on how to go about modifying the using credentials in insecure connections issue.

[gburek-fastly]

from underlying grpc framework

Your comment sparked a thought and I have found the otlphttp exporter appears to work with the linkerd provided tunnel:

extensions:
  headers_setter:
    headers:
      - key: X-Scope-OrgID
        from_context: tenant_id
      - key: User-ID
        value: user_id

receivers:
  otlp:
    protocols:
      http:
        include_metadata: true

exporters:
  otlphttp
    endpoint: otelcol:4318
    tls:
      insecure: true
    auth:
      authenticator: headers_setter

service:
  extensions: [ headers_setter ]
  pipelines:
    traces:
      receivers: [ otlp ]
      exporters: [ otlphttp ]

[andrzej-stencel]

/label -needs-triage

[pavankrish123]

The panic issue is fixed. We can close this now.

[jpkrohling]

While this might have the same underlying cause, I think this deserves a bit more attention:

opentelemetry-collector-contrib/extension/headerssetterextension/extension.go

Lines 93 to 95 in f19ae14

	func (h *headersPerRPC) RequireTransportSecurity() bool {
	return true
	}

@kovrus, do you remember whether there's a reason for us to require secure transport here? The header setter is not necessarily sending auth data, so, it shouldn't require a secure transport, should it?

[kovrus]

While this might have the same underlying cause, I think this deserves a bit more attention:

opentelemetry-collector-contrib/extension/headerssetterextension/extension.go

Lines 93 to 95 in f19ae14

	func (h *headersPerRPC) RequireTransportSecurity() bool {
	return true
	}

@kovrus, do you remember whether there's a reason for us to require secure transport here? The header setter is not necessarily sending auth data, so, it shouldn't require a secure transport, should it?

Actually, I do not remember, it probably should be safe to not require a secure transport for this extension. I'll open a PR for it.