Skip to content
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 service.name resource attribute to the collector's own telemetry #6136

Closed
jlegoff opened this issue Sep 21, 2022 · 4 comments · Fixed by #6152
Closed

Add service.name resource attribute to the collector's own telemetry #6136

jlegoff opened this issue Sep 21, 2022 · 4 comments · Fixed by #6152
Assignees

Comments

@jlegoff
Copy link

jlegoff commented Sep 21, 2022

Is your feature request related to a problem? Please describe.

The opam spec states that an agent should have the following identifying attributes:

  • service.name
  • service.namespace
  • service.version
  • service.instance.id
  • any other attributes that are necessary for uniquely identifying the Agent's own telemetry.

At the moment, the collector's own telemetry don't include these.

Describe the solution you'd like
The collector should include the above attributes - service.namespace and service.instance.id should probably be configurable. I'm not sure what the default values should be.

Describe alternatives you've considered
I've suggested in slack a specific resource type might be created for agents, but the consensus seems to be that the collector is a service.

@tigrannajaryan
Copy link
Member

At the moment, the collector's own telemetry don't include these.

This is partially correct. Collector does include service.version and service.instance.id already. service.name is indeed missing and based on today's SIG discussion should be added.

Collector also allows to specify arbitrary attributes to include in its own telemetry using resource key under the telemetry key of the config. This allows the user to define service.namespace and any other attributes and also allows overriding service.instance.id.

The only thing I believe is really missing is that service.name should be by default included in own telemetry. I would expect it to be set to the FQDN, e.g. "io.opentelemetry.collector".

@tigrannajaryan tigrannajaryan added the help wanted Good issue for contributors to OpenTelemetry Service to pick up label Sep 22, 2022
@tigrannajaryan
Copy link
Member

Some findings for posterity: I have been testing how Collector generates its own metrics using prometheusreceiver to collect them and was surprised to discover that service.name was included as Resource attributes on the collected metrics. I was surprised since I didn't expect this. However, after some digging I found that prometheusreceiver sets the service.name equal to the configured job name.

So, we still need to add the service.name as a Resource attribute, even if the prometheusreceiver overwrites it with the configured job name later (since prometheusreceiver is not the only way we will be collecting our own telemetry in the future).

@tigrannajaryan tigrannajaryan self-assigned this Sep 26, 2022
@tigrannajaryan tigrannajaryan removed the help wanted Good issue for contributors to OpenTelemetry Service to pick up label Sep 26, 2022
@tigrannajaryan tigrannajaryan changed the title Add resource attributes to the collector's own telemetry Add service.name resource attribute to the collector's own telemetry Sep 26, 2022
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-collector that referenced this issue Sep 26, 2022
Resolves open-telemetry#6136

- The default service.name is set to "io.opentelemetry.collector". I think this
  is a good choice since it is unambiguous and is still short enough to be readable.
  This also coincides with OpAMP recommendations: https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agentdescriptionidentifying_attributes
- I made the service.name a setting in BuildInfo which can be overridden when building the
  Collector. Other distros may choose a different service.name.
- The service.name can be also overridden by the end user via telemetry config setting.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-collector that referenced this issue Sep 26, 2022
Resolves open-telemetry#6136

- I set the default service.name to "io.opentelemetry.collector". I think this
  is a good choice since it is unambiguous and is still short enough to be readable.
  This also coincides with OpAMP recommendations: https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agentdescriptionidentifying_attributes
- I made the service.name a setting in BuildInfo which can be overridden when building the
  Collector. Other distros may choose a different service.name.
- The service.name can be also overridden by the end user via telemetry config setting.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-collector that referenced this issue Sep 26, 2022
Resolves open-telemetry#6136

- The default service.name is set to "io.opentelemetry.collector". I think this
  is a good choice since it is unambiguous and is still short enough to be readable.
  This also matches the OpAMP recommendations: https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agentdescriptionidentifying_attributes
- I made the service.name a setting in BuildInfo which can be overridden when building the
  Collector. Other distros may choose a different service.name.
- The service.name can be also overridden by the end user via telemetry config setting.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-collector that referenced this issue Sep 26, 2022
Resolves open-telemetry#6136

- The default service.name is set to "io.opentelemetry.collector". I think this
  is a good choice since it is unambiguous and is still short enough to be readable.
  This also matches the OpAMP recommendations: https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agentdescriptionidentifying_attributes
- I made the service.name a setting in BuildInfo which can be overridden when building the
  Collector. Other distros may choose a different service.name.
- The service.name can be also overridden by the end user via telemetry config setting.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-collector that referenced this issue Sep 26, 2022
Resolves open-telemetry#6136

- The default service.name is set to "io.opentelemetry.collector". I think this
  is a good choice since it is unambiguous and is still short enough to be readable.
  This also matches the OpAMP recommendations: https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agentdescriptionidentifying_attributes
- I made the service.name a setting in BuildInfo which can be overridden when building the
  Collector. Other distros may choose a different service.name.
- The service.name can be also overridden by the end user via telemetry config setting.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-collector that referenced this issue Sep 27, 2022
Resolves open-telemetry#6136

- The default service.name is set to "io.opentelemetry.collector". I think this
  is a good choice since it is unambiguous and is still short enough to be readable.
  This also matches the OpAMP recommendations: https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agentdescriptionidentifying_attributes
- The service.name can be overridden by the end user via telemetry config setting.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-collector that referenced this issue Sep 27, 2022
Resolves open-telemetry#6136

- The default service.name is set to "io.opentelemetry.collector". I think this
  is a good choice since it is unambiguous and is still short enough to be readable.
  This also matches the OpAMP recommendations: https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agentdescriptionidentifying_attributes
- The service.name can be overridden by the end user via telemetry config setting.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-collector that referenced this issue Sep 27, 2022
Resolves open-telemetry#6136

- The default service.name is set to "io.opentelemetry.collector". I think this
  is a good choice since it is unambiguous and is still short enough to be readable.
  This also matches the OpAMP recommendations: https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agentdescriptionidentifying_attributes
- The service.name can be overridden by the end user via telemetry config setting.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-collector that referenced this issue Sep 27, 2022
Resolves open-telemetry#6136

- The default service.name is set to "io.opentelemetry.collector". I think this
  is a good choice since it is unambiguous and is still short enough to be readable.
  This also matches the OpAMP recommendations: https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agentdescriptionidentifying_attributes
- The service.name can be overridden by the end user via telemetry config setting.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-collector that referenced this issue Sep 27, 2022
Resolves open-telemetry#6136

- The default service.name is set to "io.opentelemetry.collector". I think this
  is a good choice since it is unambiguous and is still short enough to be readable.
  This also matches the OpAMP recommendations: https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agentdescriptionidentifying_attributes
- The service.name can be overridden by the end user via telemetry config setting.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-collector that referenced this issue Sep 27, 2022
Resolves open-telemetry#6136

- The default service.name is set to "io.opentelemetry.collector". I think this
  is a good choice since it is unambiguous and is still short enough to be readable.
  This also matches the OpAMP recommendations: https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agentdescriptionidentifying_attributes
- The service.name can be overridden by the end user via telemetry config setting.
@jack-berg
Copy link
Member

The only thing I believe is really missing is that service.name should be by default included in own telemetry. I would expect it to be set to the FQDN, e.g. "io.opentelemetry.collector".

I don't think it makes sense to use a constant service.name in the collector. Perhaps a constant can be used as a fallback, but ultimately I think its a user concern to assign a logical name to the collector as they see fit. Perhaps a new property can be added to the collector YAML config?

It seems like the desire to use a constant manifests from opamp's usage of service.name as an agent type identifier. I think service.name is being incorrectly used in this regard, and opened opamp-spec #131 to discuss it.

@tigrannajaryan
Copy link
Member

I don't think it makes sense to use a constant service.name in the collector. Perhaps a constant can be used as a fallback, but ultimately I think its a user concern to assign a logical name to the collector as they see fit. Perhaps a new property can be added to the collector YAML config?

Yes, that's what the PR does. We define a default is which can be overridden by the end user using the config file.

tigrannajaryan added a commit to tigrannajaryan/opentelemetry-collector that referenced this issue Oct 3, 2022
Resolves open-telemetry#6136

- The default service.name is set to BuildInfo.Command (equals to "otelcol" for this repo).
- The service.name can be overridden by the end user via telemetry config setting.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-collector that referenced this issue Oct 3, 2022
Resolves open-telemetry#6136

- The default service.name is set to BuildInfo.Command (equals to "otelcol" for this repo).
- The service.name can be overridden by the end user via telemetry config setting.
bogdandrutu pushed a commit that referenced this issue Oct 3, 2022
…6152)

Resolves #6136

- The default service.name is set to BuildInfo.Command (equals to "otelcol" for this repo).
- The service.name can be overridden by the end user via telemetry config setting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants