-
Notifications
You must be signed in to change notification settings - Fork 890
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
add document defining an OpenTelemetry Collector #4313
base: main
Are you sure you want to change the base?
add document defining an OpenTelemetry Collector #4313
Conversation
Related to open-telemetry#4309 Signed-off-by: Alex Boten <[email protected]>
specification/collector/README.md
Outdated
|
||
- An OpenTelemetry Collector _MUST_ accept a OpenTelemetry Collector Config file. | ||
- An OpenTelemetry Collector _MUST_ be able to be compiled with any and all | ||
additional Collector plugins that the user wishes to include. |
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 this specification define what an OpenTelemetry Collector plugin is? Is it any component of type receiver, processor, exporter, extension, or config map provider?
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.
Specification needs to define all the terms used in this definition, otherwise it does not remove ambiguity.
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.
Renamed plugin to component and added a section to define OpenTelemetry Collector component.
specification/collector/README.md
Outdated
- A compiled instance of an OpenTelemetry Collector – with a specific set of | ||
plugins and features – is referred to as an OpenTelemetry Collector Distro. |
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.
Will another section contain the initial set of required plugins and features?
Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
|
||
- An OpenTelemetry Collector _MUST_ accept as OpenTelemetry Collector Configuration | ||
file. | ||
- An OpenTelemetry Collector _MUST_ be able to be compiled with any and all |
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.
Do we want to emphasize "compiled"? I think the specification is trying to avoid being too specific to the actual implementation, maybe reword this to "MUST be able to be used with any additional ...".
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.
Does this ultimately come down to requiring that an OpenTelemetry Collector
must be Open Source? There is no binary plugin mechanism on the horizon so the only way for a user to bring new components is to build a new collector, which means they need the source and license to modify it to include new components.
Co-authored-by: Reiley Yang <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Co-authored-by: Reiley Yang <[email protected]>
## OpenTelemetry Collector components | ||
|
||
For a library to be considered an OpenTelemetry Collector component, it _MUST_ | ||
implement the [Component interface](https://github.com/open-telemetry/opentelemetry-collector/blob/main/component/component.go) |
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.
nit: link to the vanity link instead of a specific file branch in a specific repo? Or perhaps this? https://pkg.go.dev/go.opentelemetry.io/collector/component#Component
additional [Collector components](#opentelemetry-collector-components) that | ||
the user wishes to include. | ||
|
||
## OpenTelemetry Collector configuration file |
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.
Open points:
- Should we require a specific schema? We'd need to define a schema for this.
- Should we require a vaguely defined format, such as "OTel Collector YAML configuration file, as documented in the examples available on the website"?
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.
I think a schema is required, at least for the shape of the configuration to the point where individual component configuration shapes become relevant. My understanding of the intent of this requirement is to enable interoperability between collectors, which requires clarity and precision about what must be accepted in a configuration file.
For a library to be considered an OpenTelemetry Collector component, it _MUST_ | ||
implement the [Component interface](https://github.com/open-telemetry/opentelemetry-collector/blob/main/component/component.go) |
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 Collector also accepts confmap.Provider
s and confmap.Converter
s, which do not accept this interface. Do we consider those out of scope?
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.
I think they do need to be considered in scope. Interoperability of those components is important.
## OpenTelemetry Collector Distribution | ||
|
||
An OpenTelemetry Collector Distribution (Distro) is a compiled instance | ||
of an OpenTelemetry Collector with a specific set of components and features. |
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 made sense as part of the short definition we had at the original issue, but here, it feels a bit strange: it might give the impression that a distribution doesn't have to enable users to include their own components, which is not in the spirit of the definition we discussed, right?
I'd propose to move the two requirements from the top of the doc here to this part:
- An OpenTelemetry Collector _MUST_ accept an [OpenTelemetry Collector configuration
file](#opentelemetry-collector-configuration-file).
- An OpenTelemetry Collector _MUST_ be able to be compiled with any and all
additional [Collector components](#opentelemetry-collector-components) that
the user wishes to include.
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.
I think there needs to be room for a compiled binary distribution that has a fixed set of components and features. The ADOT collector is only the ADOT collector when it has the set of components and features that are included in the binary that AWS produces and supports. Users can certainly fork the repo and modify the component factories to include any components they wish, but at that point it stops being the ADOT collector and is something else.
## OpenTelemetry Collector Distribution | ||
|
||
An OpenTelemetry Collector Distribution (Distro) is a compiled instance | ||
of an OpenTelemetry Collector with a specific set of components and features. |
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.
I think there needs to be room for a compiled binary distribution that has a fixed set of components and features. The ADOT collector is only the ADOT collector when it has the set of components and features that are included in the binary that AWS produces and supports. Users can certainly fork the repo and modify the component factories to include any components they wish, but at that point it stops being the ADOT collector and is something else.
|
||
- An OpenTelemetry Collector _MUST_ accept as OpenTelemetry Collector Configuration | ||
file. | ||
- An OpenTelemetry Collector _MUST_ be able to be compiled with any and all |
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.
Does this ultimately come down to requiring that an OpenTelemetry Collector
must be Open Source? There is no binary plugin mechanism on the horizon so the only way for a user to bring new components is to build a new collector, which means they need the source and license to modify it to include new components.
The goal of this document is for users to be able to easily switch between | ||
OpenTelemetry Collector Distros while also ensuring that components produced by | ||
the OpenTelemetry Collector SIG are able to work with any vendor who claims | ||
support for an OpenTelemetry Collector. |
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.
I'm not sure I understand this goal. If a vendor produces a collector distribution that has a subset of available components because those are the components relevant to their service offerings and that they're willing to support, where do any other components (whether hosted in an OTel repo or not) fit into that picture? Do we mean that a distribution must offer end users the ability to modify its source and create their own build? We should be explicit about that if that is the case.
Given that the licensing of the collector's source code does not require that distribution of derivative works happen in source form I'm not sure that we have much ability here to enforce such a requirement. We can certainly try to use the "OpenTelemetry" mark as a cudgel, but I'm not sure it'll be as effective as may be desirable since the terms "collector" and "distribution" are very broad. It could perhaps be argued that "OpenTelemetry Collector" is a protectable mark and maybe even that "Collector" has acquired secondary meaning in this limited scope, but protecting such a mark against genericization is going to be a Sisyphean task.
from: tmp/otel/specification/baggage/_index.md | ||
to: baggage/README.md |
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.
from: tmp/otel/specification/baggage/_index.md | |
to: baggage/README.md | |
from: tmp/otel/specification/collector/_index.md | |
to: collector/README.md |
additional [Collector components](#opentelemetry-collector-components) that | ||
the user wishes to include. | ||
|
||
## OpenTelemetry Collector configuration file |
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.
I think a schema is required, at least for the shape of the configuration to the point where individual component configuration shapes become relevant. My understanding of the intent of this requirement is to enable interoperability between collectors, which requires clarity and precision about what must be accepted in a configuration file.
For a library to be considered an OpenTelemetry Collector component, it _MUST_ | ||
implement the [Component interface](https://github.com/open-telemetry/opentelemetry-collector/blob/main/component/component.go) |
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.
I think they do need to be considered in scope. Interoperability of those components is important.
Changes
Adds a definition of an OpenTelemetry Collector