-
Couldn't load subscription status.
- Fork 76
Cleaning up docs for HTTP traces #859
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| The `frontend` container in the `docker-compose.yaml` file will automatically run on startup and migrate the databases if any changes are required, however administrators may wish to migrate their databases before upgrading the rest of the system when working with large databases. Sourcegraph guarantees database backward compatibility to the most recent minor point release so the database can safely be upgraded before the application code. | ||
| To execute the database migrations independently, follow the [docker-compose instructions on how to manually run database migrations](/admin/updates/migrator/migrator-operations#docker-compose). Running the `up` (default) command on the `migrator` of the *version you are upgrading to* will apply all migrations required by the next version of Sourcegraph. | ||
| To execute the database migrations independently, follow the [docker-compose instructions on how to manually run database migrations](/admin/updates/migrator/migrator-operations#docker-compose). Running the `up` (default) command on the `migrator` of the *version you are upgrading to* will apply all migrations required by that version of Sourcegraph. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This language could maybe be a little misleading in the case of customers that have to perform a multi version upgrade
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted, reverting this change, as it is not material for the tracing docs change
|
|
||
| ### Deploy OpenTelemetry Collector | ||
| 1. Deploy our bundled OpenTelemetry Collector with our bundled Jaeger backend, or | ||
| 2. Deploy our bundled OpenTelemetry Collector and configure an external tracing backend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically a customer wouldn't have to use our bundled otel collector service. A customer I'm working with is planning to use their existing otel service running in a shared cluster for their Sourcegraph deplyoment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted. That's a use case which wasn't addressed in the previous version of the docs either, and can be addressed in follow up PRs.
| To enable tracing on your Helm instance, you'll need to either: | ||
|
|
||
| Sourcegraph currently supports exporting tracing data to several backends. Refer to [OpenTelemetry](/admin/observability/opentelemetry) for detailed descriptions on how to configure your backend of choice. | ||
| 1. Deploy our bundled Jaeger backend, or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
noticed the language here is different than in the kustomize docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it is a little different as as result of the Kustomize repo providing different approaches to this from the Helm repo. The Helm chart deploys our bundled otel-collector by default, so no action required for the customer admin, whereas the Kustomize repo reads like it's an action required for the admin.
|
|
||
| #### Enable the bundled Jaeger deployment | ||
|
|
||
| Sourcegraph bundles a Jaeger instance, but it is disabled by default. You can enable it by either adding this to your Helm values override file, or by appending the [jaeger/override.yaml](https://github.com/sourcegraph/deploy-sourcegraph-helm/blob/main/charts/sourcegraph/examples/jaeger/override.yaml) file to your Helm upgrade command. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean by appending this file to the helm upgrade command? To my knowledge, all of our other helm config changes are made by direct edits to the instance's values override.yaml file, so I would think it would make sense to only suggest that option for consistency with the rest of our instructions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The helm upgrade --install command can take any number of -f values.yaml, -f override.yaml, -f jaeger/override.yaml args, and merge them together, so I created the jaeger/override.yaml file in the Helm repo to match how we're providing this option in the Docker Compose repo.
| If you require a TLS connection to export trace data, you need to first add the certificate data to a secret. The following snippet demonstrates how you can achieve this: | ||
|
|
||
| <Callout type="warning">Do NOT commit the secret manifest into your Git repository unless you are okay with storing sensitive information in plaintext and your repository is private.</Callout> | ||
| > Do NOT commit the secret manifest into your Git repository unless you are okay with storing sensitive information in plaintext and your repository is private. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any specific reason you removed the callout element here? I believe we're supposed to use these MDX elements over the quote character (per instructions in the README)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specific reason was just to maintain consistency on the page, we can run through the docs to find and replace the quote format with the callout format in a follow up PR.
| > NOTE: In case you require an additional exporter from the [`opentelemetry-collector-contrib` repository](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/exporter), please [open an issue](https://github.com/sourcegraph/sourcegraph/issues). | ||
| Basic configuration for each tracing backend type is described below. | ||
|
|
||
| > NOTE: If you require an additional exporter from the [opentelemetry-collector-contrib](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/exporter) repository, please contact Sourcegraph Customer Support. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should use <Callout type="note">...</Callout> here over quote character
| To reduce the volume of traces exported, the collector can be configured to apply sampling. Sourcegraph includes the [probabilistic](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/probabilisticsamplerprocessor) and [tail](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/processor/tailsamplingprocessor/README) samplers in the bundled collector. | ||
| If enabled, this sampling mechanism will be applied to all traces, regardless if a request was explictly marked as to be traced. | ||
| > NOTE: If sampling is enabled, the sampling mechanism will be applied to all traces, regardless if a request was explicitly requested to be traced. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should use <Callout type="note">...</Callout> here over quote character
| This section outlines some common exporter configurations. For details, see OpenTelemetry's [exporters](https://opentelemetry.io/docs/collector/configuration/#exporters) page. | ||
| > NOTE: In case you require an additional exporter from the [`opentelemetry-collector-contrib` repository](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/exporter), please [open an issue](https://github.com/sourcegraph/sourcegraph/issues). | ||
| > NOTE: If you require an additional exporter from the [opentelemetry-collector-contrib](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/exporter) repository, please contact Sourcegraph Customer Support. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should use <Callout type="note">...</Callout> here over quote character
| If you would like to make changes to the default configurations, we highly recommend you to create a new file called `docker-compose.override.yaml` in the same directory where the base file ([docker-compose.yaml](https://github.com/sourcegraph/deploy-sourcegraph-docker/blob/master/docker-compose/docker-compose.yaml)) is located, and make your customizations inside the `docker-compose.override.yaml` file. | ||
|
|
||
| >WARNING: For configuration of Sourcegraph, see Sourcegraph's [configuration](/admin/config/) docs. | ||
| To configure your Sourcegraph instance, see Sourcegraph's [configuration](/admin/config/) docs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use <Callout type="warning">...</Callout> here
| - '~/sg/volume-maps/gitserver/.ssh:/home/sourcegraph/.ssh' | ||
| ``` | ||
| > WARNING: The permissions on your SSH / Git configuration must be set to be readable by the user in the `gitserver` container. For example, run `chmod -v -R 600 ~/sg/volume-maps/gitserver/.ssh` in the folder on the host machine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use <Callout type="warning">...</Callout> here over quote character
Centralizing, deduplicating, and correcting information about HTTP traces from multiple different pages into two primary pages specifically about Tracing and OpenTelemetry, then minimizing the content on the deployment / config pages to link back to these two, only leaving the deployment-type-specific details on the pages for each deployment-type.
Pull Request approval
You will need to get your PR approved by at least one member of the Sourcegraph team. For reviews of docs formatting, styles, and component usage, please tag the docs team via the #docs Slack channel.