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

cmd/server: remove jaeger, disable opentelemetry by default#41244

Merged
bobheadxi merged 5 commits intomainfrom
src-server-remove-jaeger
Sep 2, 2022
Merged

cmd/server: remove jaeger, disable opentelemetry by default#41244
bobheadxi merged 5 commits intomainfrom
src-server-remove-jaeger

Conversation

@bobheadxi
Copy link
Member

@bobheadxi bobheadxi commented Sep 2, 2022

With jaeger no longer being the default tracing mechanism (https://github.com/sourcegraph/sourcegraph/pull/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 (https://github.com/sourcegraph/sourcegraph/pull/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.

Docs indicating it is not recommended for production use:

image

Test plan

CI passes, main-dry-run: https://buildkite.com/sourcegraph/sourcegraph/builds?branch=main-dry-run/src-server-remove-jaeger

@cla-bot cla-bot bot added the cla-signed label Sep 2, 2022
@bobheadxi bobheadxi requested review from a team September 2, 2022 02:51
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Sep 2, 2022

Codenotify: Notifying subscribers in CODENOTIFY files for diff af067ef...8b4ec25.

Notify File(s)
@sourcegraph/delivery doc/admin/deploy/docker-single-container/index.md

@bobheadxi bobheadxi requested review from a team, jhchabran and sanderginn September 2, 2022 03:15
Copy link
Contributor

@sanderginn sanderginn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a blurb to stress that while this deployment ships without tracing we do in fact support it.

For the rest, LGTM

The Docker Single Container deployment type is a way to very quickly get an instance of Sourcegraph set up locally to experiment with many of its features. However, it is **not recommended** for a production instance, and **has limitations** depending on the OS you are deploying to, as well as the associated resources. See the [troubleshooting section](#troubleshooting) for additional information.

[Code Insights](../../../code_insights/index.md) is not supported in Single Container deployments. To try Code Insights you must deploy using [Docker Compose](../docker-compose/index.md) or [Kubernetes](../kubernetes/index.md).
[Code Insights](../../../code_insights/index.md) is not supported in Single Container deployments. To try Code Insights you must deploy using [Docker Compose](../docker-compose/index.md) or [Kubernetes](../kubernetes/index.md). [Tracing](../../observability/tracing.md) is disabled by default, and if you intend to enable it, you will have to deploy and configure the [OpenTelemetry Collector](../../observability/opentelemetry.md). The Single Container deployment does not ship with this service included. It is strongly recommended to use one of the aforementioned deployment methods if tracing support is a requirement.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realise this is quite the understatement of the amount of effort it would take someone to get this all to work but I don't think anyone will actually attempt it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, thanks for the addition!

@bobheadxi bobheadxi merged commit ac22d5d into main Sep 2, 2022
@bobheadxi bobheadxi deleted the src-server-remove-jaeger branch September 2, 2022 15:16
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>
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.

4 participants