Add tracing service and configuration #12699

rosstimothy · 2022-05-17T13:21:53Z

Provides a new tracing configuration block, which can be
used to configure if and how spans are exported to a
telemetry backend. In the example below, the tracing
service is enabled and will export spans to
collector.example.com:4317 via gRPC with mTLS enabled.

tracing_service:
  enabled: yes
  exporter_url: collector.example.com:4317
  sampling_rate_per_million: 1000000
  ca_certs:
    - /certs/rootCA.pem
  keypairs:
    - key_file:  /certs/example.com-client-key.pem
      cert_file: /certs/example.com-client.pem

This configuration ends up being consumed by the TeleportProcess
and passed to tracing.NewTraceProvider which sets up the OpenTelemetry
Exporter, TracerProvider, Propagator and Sampler. In order for spans to
be exported, the tracing_service must be enabled and have a
sampling_rate_per_million value > 0.

#12241

lib/config/configuration.go

lib/observability/tracing/tracing.go

espadolini · 2022-05-23T11:46:42Z

lib/observability/tracing/tracing.go

+		return trace.BadParameter("exporter URL cannot be empty")
+	}
+
+	if !strings.Contains(c.ExporterURL, "://") {


I'd prefer a proper URL parse and then a check on Scheme here.

I initially tried doing this; however url.Parse("localhost:4317") will result in a url with the Scheme set to localhost (https://go.dev/play/p/VoYSAAhscPQ). Open to other ideas if you have them

We could either try to parse it as an address first and then as a URL on error, or we could just always require a URL, and whatever error happens ("unsupported schema localhost:") is the user's fault.

My goal here was to have an empty scheme default to grpc. This seems to be how most other tracing exporters operate. Forcing users to always provide one of grpc, http, https would allow us to just do a url.Parse here but might be annoying to users. I suppose that would be fine as long as I add that grpc:// is required in the yet to exist tracing docs.

To identify that no Scheme was provided, we could check if the Host is empty after parsing the URL (https://go.dev/play/p/K_m82MHG1OO). As the documentation states:

URLs that do not start with a slash after the scheme are interpreted as:
scheme:opaque[?query][#fragment]

After knowing no scheme was provided, having a default scheme is valuable if it is the most used, but it should also be documented.

lib/observability/tracing/tracing.go

espadolini · 2022-05-23T12:04:48Z

lib/observability/tracing/tracing.go

+		sdktrace.WithResource(res),
+		sdktrace.WithSpanProcessor(sdktrace.NewBatchSpanProcessor(exporter)),
+	)}
+	otel.SetTracerProvider(provider)


What happens during tests or during a CA rotation, when the global provider might get overwritten?

Hmm good question. I will test this and possibly add a check to see if the global provider is already configured correctly

In most cases the global provider should only be used at creation time to provide a component specific tracer.

Could we perhaps run the tracing configuration earlier than other components, and store this as a variable in TeleportProcess rather than storing it globally? Or is otel really only supposed to work with a single global exporter for the whole program?

I think we should do both. The default global provider is a noop which won't record any spans. I'm explicitly setting it here so that in the event a provider can't be provided as a dependency we can fallback to retrieving it from the global provider. Though I hope we can avoid using the global provider as much as possible

lib/service/service.go

lib/config/configuration.go

espadolini · 2022-05-25T08:26:59Z

lib/observability/tracing/tracing.go

+		return trace.BadParameter("exporter URL cannot be empty")
+	}
+
+	if !strings.Contains(c.ExporterURL, "://") {


We could either try to parse it as an address first and then as a URL on error, or we could just always require a URL, and whatever error happens ("unsupported schema localhost:") is the user's fault.

lib/observability/tracing/tracing.go

espadolini · 2022-05-25T08:39:25Z

lib/observability/tracing/tracing.go

+		sdktrace.WithResource(res),
+		sdktrace.WithSpanProcessor(sdktrace.NewBatchSpanProcessor(exporter)),
+	)}
+	otel.SetTracerProvider(provider)


Could we perhaps run the tracing configuration earlier than other components, and store this as a variable in TeleportProcess rather than storing it globally? Or is otel really only supposed to work with a single global exporter for the whole program?

lib/service/cfg.go

espadolini · 2022-05-25T08:43:10Z

lib/service/cfg.go

+	// KeyPairs are the key and certificate pairs that the tracing service will.
+	// use for mTLS.


Suggested change

// KeyPairs are the key and certificate pairs that the tracing service will.

// use for mTLS.

// KeyPairs are the paths for key and certificate pairs that the tracing service will use for outbound TLS connections.

Should we ask Cloud if they'd like a way to specify these inline in the configuration file?

Cloud actually designed and implemented the metrics service and since this is a direct copy of that I don't think they will have any objections. I can ping them to be certain though

espadolini · 2022-05-25T08:46:05Z

lib/service/service.go

+				log.Info("Shutting down immediately.")
+				ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+				defer cancel()
+				warnOnErr(provider.Shutdown(ctx), log)


FYI the scram shutdown (on SIGINT/SIGTERM) will not wait for services to shutdown, so it's very likely that we won't have the full 5 seconds to perform the provider shutdown - is that going to be a problem?

Any queued spans which haven't been flushed yet will be lost

Provides a new tracing configuration block, which can be used to configure if and how spans are exported to a telemetry backend. In the example below, the tracing service is enabled and will export spans to `collector.example.com:4317` via gRPC with mTLS enabled. ```yaml tracing_service: enabled: yes exporter_url: collector.example.com:4317 sampling_rate_per_million: 1000000 ca_certs: - /certs/rootCA.pem keypairs: - key_file: /certs/example.com-client-key.pem cert_file: /certs/example.com-client.pem ``` This configuration ends up being consumed by the `TeleportProcess` and passed to `tracing.NewTraceProvider` which sets up the OpenTelemetry Exporter, TracerProvider, Propagator and Sampler. In order for spans to be exported, the `tracing_service` must be enabled **and** have a `sampling_rate_per_million` value > 0.

* Add tracing service and configuration Provides a new tracing configuration block, which can be used to configure if and how spans are exported to a telemetry backend. In the example below, the tracing service is enabled and will export spans to `collector.example.com:4317` via gRPC with mTLS enabled. ```yaml tracing_service: enabled: yes exporter_url: collector.example.com:4317 sampling_rate_per_million: 1000000 ca_certs: - /certs/rootCA.pem keypairs: - key_file: /certs/example.com-client-key.pem cert_file: /certs/example.com-client.pem ``` This configuration ends up being consumed by the `TeleportProcess` and passed to `tracing.NewTraceProvider` which sets up the OpenTelemetry Exporter, TracerProvider, Propagator and Sampler. In order for spans to be exported, the `tracing_service` must be enabled **and** have a `sampling_rate_per_million` value > 0.

Add tracing service and configuration (gravitational#12699) * Add tracing service and configuration Provides a new tracing configuration block, which can be used to configure if and how spans are exported to a telemetry backend. In the example below, the tracing service is enabled and will export spans to `collector.example.com:4317` via gRPC with mTLS enabled. ```yaml tracing_service: enabled: yes exporter_url: collector.example.com:4317 sampling_rate_per_million: 1000000 ca_certs: - /certs/rootCA.pem keypairs: - key_file: /certs/example.com-client-key.pem cert_file: /certs/example.com-client.pem ``` This configuration ends up being consumed by the `TeleportProcess` and passed to `tracing.NewTraceProvider` which sets up the OpenTelemetry Exporter, TracerProvider, Propagator and Sampler. In order for spans to be exported, the `tracing_service` must be enabled **and** have a `sampling_rate_per_million` value > 0.

rosstimothy force-pushed the tross/tracing_config branch 2 times, most recently from 5d81950 to 8277251 Compare May 17, 2022 16:22

rosstimothy marked this pull request as ready for review May 17, 2022 17:14

github-actions bot requested review from espadolini and gabrielcorado May 17, 2022 17:15

espadolini reviewed May 23, 2022

View reviewed changes

rosstimothy force-pushed the tross/tracing_config branch 2 times, most recently from e90bff2 to 69376c7 Compare May 23, 2022 20:04

espadolini reviewed May 25, 2022

View reviewed changes

rosstimothy added 3 commits May 25, 2022 11:29

review feedback

fb3a06c

clean up tls config and add provider to TeleportProcess

b8332b7

rosstimothy force-pushed the tross/tracing_config branch from 69376c7 to b8332b7 Compare May 25, 2022 15:29

gabrielcorado approved these changes May 26, 2022

View reviewed changes

espadolini approved these changes May 26, 2022

View reviewed changes

rosstimothy and others added 4 commits May 26, 2022 15:42

better exporter url parsing and defaults

80c6c9e

Merge branch 'master' into tross/tracing_config

4fe48d3

Merge branch 'master' into tross/tracing_config

deaa851

Merge branch 'master' into tross/tracing_config

dd2b67d

rosstimothy enabled auto-merge (squash) May 26, 2022 22:35

rosstimothy merged commit 5b4a18b into master May 26, 2022

This was referenced Aug 5, 2022

[v9] Add tracing service and configuration #15243

Merged

[v8] Add tracing service and configuration #15244

Merged

rosstimothy deleted the tross/tracing_config branch January 5, 2023 20:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tracing service and configuration #12699

Add tracing service and configuration #12699

rosstimothy commented May 17, 2022

espadolini May 23, 2022

rosstimothy May 23, 2022

espadolini May 25, 2022

rosstimothy May 25, 2022

gabrielcorado May 26, 2022

espadolini May 23, 2022

rosstimothy May 23, 2022

rosstimothy May 23, 2022

espadolini May 25, 2022

rosstimothy May 25, 2022 •

edited

Loading

espadolini May 25, 2022

espadolini May 25, 2022

espadolini May 25, 2022

espadolini May 25, 2022

rosstimothy May 25, 2022

espadolini May 25, 2022

rosstimothy May 25, 2022

		// KeyPairs are the key and certificate pairs that the tracing service will.
		// use for mTLS.

	// KeyPairs are the key and certificate pairs that the tracing service will.
	// use for mTLS.
	// KeyPairs are the paths for key and certificate pairs that the tracing service will use for outbound TLS connections.

Add tracing service and configuration #12699

Add tracing service and configuration #12699

Conversation

rosstimothy commented May 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rosstimothy May 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rosstimothy May 25, 2022 •

edited

Loading