Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

tracing: enable opentelemetry by default, remove legacy values#41242

Merged
bobheadxi merged 14 commits into
mainfrom
otel-default-tracing
Sep 7, 2022
Merged

tracing: enable opentelemetry by default, remove legacy values#41242
bobheadxi merged 14 commits into
mainfrom
otel-default-tracing

Conversation

@bobheadxi

@bobheadxi bobheadxi commented Sep 2, 2022

Copy link
Copy Markdown
Member

Now that all our deployment methods bundle OpenTelemetry Collector by default (https://github.com/sourcegraph/sourcegraph/issues/40456, https://github.com/sourcegraph/sourcegraph/issues/40457, https://github.com/sourcegraph/sourcegraph/issues/40455), we can now switch over to enabling OpenTelemetry by default. This change also:

  • removes the default traceURL template, since the Jaeger instance it points to by default no longer exists on most deployments.
  • fixes the behaviour of observability.tracing config, and adds testing
  • refactors some things in internal/trace to be OpenTelemetry-first

Closes https://github.com/sourcegraph/sourcegraph/issues/39399

Test plan

CI passes, manual testing

@cla-bot cla-bot Bot added the cla-signed label Sep 2, 2022
@bobheadxi bobheadxi force-pushed the otel-default-tracing branch from a95a877 to 0370a5a Compare September 2, 2022 03:05
bobheadxi added a commit that referenced this pull request Sep 2, 2022
With jaeger no longer being the default tracing mechanism (#41242), this change removes jaeger from sourcegraph/server - it is disrecommended for production use, so I think this is probably fine - and also disables OpenTelemetry (#41243) in sourcegraph/server so we don't have to figure out an OpenTelemetry deployment for it (again, this image is not recommended for production use so I don't think we need to introduce advanced capabilities like OpenTelemetry out-of-the-box for it (there is some precedent for this - cAdvisor, for example, is not included in sourcegraph/server). Users can still configure an exporter with OTEL_EXPORTER_OTLP_ENDPOINT.

Co-authored-by: Sander Ginn <sander@ginn.it>
efritz pushed a commit that referenced this pull request Sep 6, 2022
With jaeger no longer being the default tracing mechanism (#41242), this change removes jaeger from sourcegraph/server - it is disrecommended for production use, so I think this is probably fine - and also disables OpenTelemetry (#41243) in sourcegraph/server so we don't have to figure out an OpenTelemetry deployment for it (again, this image is not recommended for production use so I don't think we need to introduce advanced capabilities like OpenTelemetry out-of-the-box for it (there is some precedent for this - cAdvisor, for example, is not included in sourcegraph/server). Users can still configure an exporter with OTEL_EXPORTER_OTLP_ENDPOINT.

Co-authored-by: Sander Ginn <sander@ginn.it>
@bobheadxi bobheadxi marked this pull request as ready for review September 6, 2022 17:52
@bobheadxi bobheadxi requested a review from a team September 6, 2022 17:52
@sourcegraph-bot

sourcegraph-bot commented Sep 6, 2022

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 86602f9...e7b1d34.

Notify File(s)
@keegancsmith internal/trace/url.go
internal/tracer/switchable_ot.go
internal/tracer/switchable_otel.go
internal/tracer/tracer.go
internal/tracer/watch.go
internal/tracer/watch_test.go
@sourcegraph/delivery doc/admin/observability/opentelemetry.md
doc/admin/observability/tracing.md
@sourcegraph/dev-experience internal/trace/url.go
internal/tracer/switchable_ot.go
internal/tracer/switchable_otel.go
internal/tracer/tracer.go
internal/tracer/watch.go
internal/tracer/watch_test.go

@bobheadxi bobheadxi enabled auto-merge (squash) September 7, 2022 17:19
@bobheadxi bobheadxi merged commit 382be7f into main Sep 7, 2022
@bobheadxi bobheadxi deleted the otel-default-tracing branch September 7, 2022 17:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

site-config: make otel the default trace export method

4 participants