fix: style compliance, tense, capitalization #8420
Conversation
Signed-off-by: Shubham Kumar <[email protected]>
otelbot-docs
bot
requested review from
a team and
dmitryax
and removed request for
a team
Signed-off-by: Shubham Kumar <[email protected]>
shubhkr72
force-pushed
the
fix/custom-collector
branch
from
873f7ec to
0bf970e
Compare
Signed-off-by: Shubham Kumar <[email protected]>
Signed-off-by: Shubham Kumar <[email protected]>
Signed-off-by: Shubham Kumar <[email protected]>
chalin
left a comment
chalin
left a comment
There was a problem hiding this comment.
Thanks for the PR @shubhkr72.
Drive-by question: why are you inlining {{% version-from-registry ... %}} directives? You wrote "added dynamic version shortcodes version-from-registry". Inlining doesn't seem dynamic to me :).
/cc @tiffany76
tiffany76
left a comment
tiffany76
left a comment
There was a problem hiding this comment.
Hi @shubhkr72! Thanks for doing this copy edit! ✨
This page needs more work than some, so you'll see that I've taken your edits a bit further. This is a partial first pass review. I'll come back to review further as soon as I can. Thanks again!
| The OpenTelemetry Community developed a tool called [OpenTelemetry Collector | ||
| builder][ocb] (or `ocb` for short) to assist people in assembling their own | ||
| distribution, making it easy to build a distribution that includes their custom | ||
| components along with components that are publicly available. |
There was a problem hiding this comment.
Let's shorten this paragraph a little.
You can use the [OpenTelemetry Collector
Builder][ocb] (or ocb for short) to assemble your own
distribution that includes custom
components, upstream components, and other publicly available components.
| ## Step 1 - Install the builder | ||
|
|
||
| {{% alert color="primary" title="Note" %}} | ||
| {{% alert title="Note" %}} |
There was a problem hiding this comment.
I think we should pull this content out of an admonition and make a new section called ## Prerequisites that comes before the steps.
| title: Building a custom collector | ||
| weight: 29 | ||
| # prettier-ignore | ||
| cSpell:ignore: chipset darwin debugexporter gomod otlpexporter otlpreceiver wyrtw |
There was a problem hiding this comment.
I can't leave a comment on the specific line, but can we change the title of the page to be "Build a custom Collector with OpenTelemetry Collector Builder" And the linkTitle can be "Build a custom Collector".
| Collector [releases with `cmd/builder` tags](https://github.com/open-telemetry/opentelemetry-collector-releases/tags). You can find a list of | ||
| assets named based on OS and chipset, so download the one that fits your | ||
| configuration: |
There was a problem hiding this comment.
Please keep the link reference rather than add the URL directly. It's true our docs are inconsistent with how we handle links, but in this case, there's no reason to change it. Also, I think the last sentence can be made a little clearer.
| Collector [releases with `cmd/builder` tags](https://github.com/open-telemetry/opentelemetry-collector-releases/tags). You can find a list of | |
| assets named based on OS and chipset, so download the one that fits your | |
| configuration: | |
| Collector [releases with `cmd/builder` tags][tags]. Find and download the asset that fits your operating system and chipset: |
| assets named based on OS and chipset, so download the one that fits your | ||
| configuration: | ||
|
|
||
| {{< tabpane text=true >}} |
There was a problem hiding this comment.
I agree with @chalin that we should keep the tabbed pane section as it was, with variables for version numbers. We also don't need the bulleted list. Please revert the changes to the tabbed pane section, from line 42 to 88 in the original file. Thank you!
| As you can see in the table above, all the `dist` tags are optional. You add | ||
| custom values for them depending on whether you intend to make your | ||
| custom Collector distribution available for consumption by other users or if you |
There was a problem hiding this comment.
| As you can see in the table above, all the `dist` tags are optional. You add | |
| custom values for them depending on whether you intend to make your | |
| custom Collector distribution available for consumption by other users or if you | |
| As you can see in the table above, all the `dist` tags are optional. You add | |
| custom values for them depending on whether you intend to make your | |
| custom Collector distribution available to other users or you |
| custom values for them depending on whether you intend to make your | ||
| custom Collector distribution available for consumption by other users or if you | ||
| are simply leveraging the `ocb` to bootstrap your component development and | ||
| are simply using the `ocb` to bootstrap your component development and |
There was a problem hiding this comment.
| are simply using the `ocb` to bootstrap your component development and | |
| are using the `ocb` to bootstrap your component development and |
| testing environment. | ||
|
|
||
| For this tutorial, you will be creating a Collector's distribution to support | ||
| For this tutorial, you create a Collector's distribution to support |
There was a problem hiding this comment.
| For this tutorial, you create a Collector's distribution to support | |
| In the following example, you create a Collector's distribution to support |
| For this tutorial, you create a Collector's distribution to support | ||
| the development and testing of components. | ||
|
|
||
| Go ahead and create a manifest file named `builder-config.yaml` with the |
There was a problem hiding this comment.
| Go ahead and create a manifest file named `builder-config.yaml` with the | |
| 1. Create a manifest file named `builder-config.yaml` with the |
| Now add the modules representing the components you want | ||
| incorporated in this custom Collector distribution. Take a look at the | ||
| [ocb configuration documentation](https://github.com/open-telemetry/opentelemetry-collector/tree/main/cmd/builder#configuration) | ||
| to understand the different modules and how to add the components. | ||
| We will be adding the following components to our development and testing | ||
| collector distribution: | ||
| Add the following components to the development and testing | ||
| Collector distribution: | ||
| - Exporters: OTLP and Debug | ||
| - Receivers: OTLP | ||
| - Processors: Batch | ||
| The `builder-config.yaml` manifest file will look like this after adding the | ||
| The `builder-config.yaml` manifest file looks like this after adding the |
There was a problem hiding this comment.
I'm condensing this section a bit, but the suggestion might not work. You might have to make the changes locally.
| Now add the modules representing the components you want | |
| incorporated in this custom Collector distribution. Take a look at the | |
| [ocb configuration documentation](https://github.com/open-telemetry/opentelemetry-collector/tree/main/cmd/builder#configuration) | |
| to understand the different modules and how to add the components. | |
| We will be adding the following components to our development and testing | |
| collector distribution: | |
| Add the following components to the development and testing | |
| Collector distribution: | |
| - Exporters: OTLP and Debug | |
| - Receivers: OTLP | |
| - Processors: Batch | |
| The `builder-config.yaml` manifest file will look like this after adding the | |
| The `builder-config.yaml` manifest file looks like this after adding the | |
| 2. Add modules for the components you want | |
| to include in your custom Collector distribution. See the | |
| [ocb configuration documentation](https://github.com/open-telemetry/opentelemetry-collector/tree/main/cmd/builder#configuration) | |
| to understand the different modules and how to add components. | |
| For this example, add the following components: | |
| - Exporters: OTLP and Debug | |
| - Receivers: OTLP | |
| - Processors: Batch | |
| The `builder-config.yaml` manifest file looks like this after adding the components: |
tiffany76
added
the
sig:collector:refactor
github-project-automation
bot
moved this to Backlog
in SIG Comms + SIG Collector: Doc refactoring
tiffany76
left a comment
tiffany76
left a comment
There was a problem hiding this comment.
Okay @shubhkr72, I've completed my first review. Let us know if you have any questions!
| [OpenTelemetry Registry](/ecosystem/registry/?language=collector). Note that | ||
| registry entries provide the full name and version you need to add to your | ||
| `builder-config.yaml`. |
There was a problem hiding this comment.
| [OpenTelemetry Registry](/ecosystem/registry/?language=collector). Note that | |
| registry entries provide the full name and version you need to add to your | |
| `builder-config.yaml`. | |
| [OpenTelemetry Registry](/ecosystem/registry/?language=collector). Each | |
| registry entry contains the full name and version you need to add to your | |
| `builder-config.yaml`. |
|
|
||
| {{% /alert %}} | ||
|
|
||
| ## Step 3a - Generate the code and build your Collector's distribution |
There was a problem hiding this comment.
| ## Step 3a - Generate the code and build your Collector's distribution | |
| ## Generate the code and build your Collector distribution |
| This step builds your custom Collector distribution using the `ocb` | ||
| binary. If you would like to build and deploy your custom Collector distribution | ||
| to a container orchestrator (for example, Kubernetes), skip this step and go to | ||
| [Step 3b](#step-3b---containerize-your-collectors-distribution). |
There was a problem hiding this comment.
| This step builds your custom Collector distribution using the `ocb` | |
| binary. If you would like to build and deploy your custom Collector distribution | |
| to a container orchestrator (for example, Kubernetes), skip this step and go to | |
| [Step 3b](#step-3b---containerize-your-collectors-distribution). | |
| This section instructs you to build your custom Collector distribution using the `ocb` | |
| binary. If you would like to build and deploy your custom Collector distribution | |
| to a container orchestrator, such as Kubernetes, skip this section and see | |
| [Containerize your Collector Distribution](#step-3b---containerize-your-collector-distribution). |
| ``` | ||
|
|
||
| If everything went well, here is what the output of the command should look | ||
| If everything went well, here is what the output of the command looks |
There was a problem hiding this comment.
| If everything went well, here is what the output of the command looks | |
| The output of the command looks |
| ```nocode | ||
| 2022-06-13T14:25:03.037-0500 INFO internal/command.go:85 OpenTelemetry Collector distribution builder {"version": "{{% version-from-registry collector-builder noPrefix %}}", "date": "2023-01-03T15:05:37Z"} | ||
| ```text | ||
| 2022-06-13T14:25:03.037-0500 INFO internal/command.go:85 OpenTelemetry Collector distribution builder {"version": "0.139.0", "date": "2023-01-03T15:05:37Z"} |
There was a problem hiding this comment.
Here's another spot where we need to maintain the variable.
| COPY ./builder-config.yaml builder-config.yaml | ||
| RUN --mount=type=cache,target=/root/.cache/go-build GO111MODULE=on go install go.opentelemetry.io/collector/cmd/builder@{{% version-from-registry collector-builder %}} | ||
| RUN --mount=type=cache,target=/root/.cache/go-build GO111MODULE=on go install go.opentelemetry.io/collector/cmd/builder@v0.139.0 |
There was a problem hiding this comment.
Please keep the variable here.
| ``` | ||
| <!-- prettier-ignore-end --> | ||
|
|
||
| The following is the minimalist `collector-config.yaml` definition: |
There was a problem hiding this comment.
| The following is the minimalist `collector-config.yaml` definition: | |
| The following is a minimal `collector-config.yaml` definition: |
| # Test the newly-built image | ||
| docker run -it --rm -p 4317:4317 -p 4318:4318 \ | ||
| --name otelcol <collector_distribution_image_name>:<version> |
There was a problem hiding this comment.
On line 335 above, can you change OCB to ocb?
| [ocb]: | ||
| https://github.com/open-telemetry/opentelemetry-collector/tree/main/cmd/builder | ||
| [tags]: https://github.com/open-telemetry/opentelemetry-collector-releases/tags |
There was a problem hiding this comment.
We need to keep these link references.
|
|
||
| The OpenTelemetry Community developed a tool called [OpenTelemetry Collector | ||
| builder][ocb] (or `ocb` for short) to assist people in assembling their own | ||
| distribution, making it easy to build a distribution that includes their custom |
There was a problem hiding this comment.
On the line above, can you capitalize Builder?
tiffany76
moved this from Backlog
to In review
in SIG Comms + SIG Collector: Doc refactoring
3 participants
Description
This PR addresses issue #8361 by copy editing the "Building a custom Collector" page to ensure style guide compliance and improve clarity.
Changes made:
{{% alert %}}){{% version-from-registry %}}) for better maintainabilityWhat this improves:
The page now follows the repository's style guide consistently, reads more naturally with present tense instructions, and uses dynamic versioning to stay current with releases automatically.
Fixes #8361
Preview: https://deploy-preview-8420--opentelemetry.netlify.app/docs/collector/custom-collector/