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

Make service.instance.id and other telemetry attributes available to extensions #6599

Closed
tigrannajaryan opened this issue Nov 22, 2022 · 13 comments
Assignees
Milestone

Comments

@tigrannajaryan
Copy link
Member

tigrannajaryan commented Nov 22, 2022

OpAMP extension will need to know the generated service.instance.id and other telemetry attributes to use it in the communication with the OpAMP Server.

This can be implemented by making it available on the component.Host interface or by adding a field to xCreateSettings.

For reference the OpAMP spec requires that in OpAMP protocol the following is set:

For standalone running Agents (such as OpenTelemetry Collector) the following attributes SHOULD be specified:
- service.name should be set to the same value that the Agent uses in its own telemetry.
- service.namespace if it is used in the environment where the Agent runs.
- service.version should be set to version number of the Agent build.
- service.instance.id should be set. It may be be set equal to the Agent's instance uid (equal to ServerToAgent.instance_uid field) or any other value that uniquely identifies the Agent in combination with other attributes.
@bogdandrutu
Copy link
Member

Why do you need to know only the service.instance.id and not all the service parts? Per spec service.instance.id is not unique, the combination of the 3 service parts are unique.

@tigrannajaryan
Copy link
Member Author

tigrannajaryan commented Nov 22, 2022

Why do you need to know only the service.instance.id and not all the service parts? Per spec service.instance.id is not unique, the combination of the 3 service parts are unique.

That's a good point. I was thinking that the extension will get the service name from the BuildInfo (we set service.name to BuildInfo.Command), but this is not the right way, since we now allow it to be overriden.

So you are right, it is best that we make all telemetry attributes available to extensions. Updated the description to make this clear.

@tigrannajaryan tigrannajaryan changed the title Make generated service.instance.id available to extensions Make generated service.instance.id and other telemetry attributes available to extensions Nov 22, 2022
@tigrannajaryan tigrannajaryan changed the title Make generated service.instance.id and other telemetry attributes available to extensions Make service.instance.id and other telemetry attributes available to extensions Nov 22, 2022
@bogdandrutu
Copy link
Member

Who provides the configuration? Is not the "OpAMP"? If that is the case you have access to them... Also, they are in the config under service::telemetry::resource, so if you have access to final config you have access to them.

@tigrannajaryan
Copy link
Member Author

Who provides the configuration? Is not the "OpAMP"?

No, "opamp" extension does not provide configuration.

Also, they are in the config under service::telemetry::resource, so if you have access to final config you have access to them.

They are not in the config if the user does not specify them. In that case the Collector generates the id and sets the service name equal to BuildInfo.Command. The "opamp" extension needs access to both of these values.

To make it clear: this feature request is for the situation when there is no Supervisor. You only enable "opamp" extension and it implements a small subset of OpAMP capabilities, mostly status and config reporting. See design doc https://docs.google.com/document/d/1KtH5atZQUs9Achbce6LiOaJxLbksNJenvgvyKLsJrkc/edit

When we have the Supervisor this feature is not necessary.

@srikanthccv
Copy link
Member

I was looking for a way to access these attributes for a different reason (specifically in the processor component). @tigrannajaryan I would like to help with this. Since the CreateSettings already embeds the TelemetrySettings, it seems like a good option to provide the resource attributes with it. What do you think?

@tigrannajaryan
Copy link
Member Author

I was looking for a way to access these attributes for a different reason (specifically in the processor component). @tigrannajaryan I would like to help with this. Since the CreateSettings already embeds the TelemetrySettings, it seems like a good option to provide the resource attributes with it. What do you think?

Sounds reasonable to me, but @open-telemetry/collector-maintainers may have other thoughts on what is a good way to address this issue.

@srikanthccv
Copy link
Member

@bogdandrutu do you have any thoughts on this?

@evan-bradley
Copy link
Contributor

@bogdandrutu @codeboten I'd like your input on how we can approach this.

In regards to what attributes we make available, I think it would likely make the most sense to provide all resource attributes to extensions. While the OpAMP extension only needs the service attributes, I think other extensions may be able to make use of other resource attributes. If we don't want to provide all resource attributes, I think we should specify the scope of attributes we are willing to provide to extensions.

We can get the attributes in the service from the SDK Resource object that is used to provide resource information to the SDKs. This will also guarantee the extensions and SDKs have the same data.

As for providing this data to extensions, I think passing it under the CreateSettings struct makes the most sense since it will have the same lifecycle as the TelemetrySettings struct.

@codeboten
Copy link
Contributor

Following up on this, there was a discussion about this issue at a SIG call in February where I believe we agreed that the right way to move this forward was to make all resource attributes of the collector available.

Making them available as part of CreateSettings seems like a sensible way to accomplish this.

@jaronoff97
Copy link
Contributor

I'm planning on taking this one, could someone assign to me?

codeboten pushed a commit that referenced this issue May 8, 2023
Adds the ability for create settings to access instantiated resource attributes

Link to tracking Issue: #6599
@mx-psi
Copy link
Member

mx-psi commented May 15, 2023

Did #7531 fix this issue completely, or is there any remaining work to do here?

@tigrannajaryan
Copy link
Member Author

I haven't verified #7531 but from the description it seems like it should do all that is needed.

@mx-psi
Copy link
Member

mx-psi commented May 15, 2023

Alright, seems that way to me as well, closing this then :)

@mx-psi mx-psi closed this as completed May 15, 2023
evan-bradley pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this issue Oct 26, 2023
See [design
document](https://docs.google.com/document/d/1KtH5atZQUs9Achbce6LiOaJxLbksNJenvgvyKLsJrkc/edit#heading=h.ioikt02qpy5f).

Depends on:

- [Implement ability for extensions to be notified about effective
configuration opentelemetry-collector#6596](open-telemetry/opentelemetry-collector#6596)
- [Make service.instance.id and other telemetry attributes available to
extensions opentelemetry-collector#6599](open-telemetry/opentelemetry-collector#6599)

Closes
#16618

---------

Signed-off-by: Sean Porter <portertech@gmail.com>
Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
sigilioso pushed a commit to carlossscastro/opentelemetry-collector-contrib that referenced this issue Oct 27, 2023
See [design
document](https://docs.google.com/document/d/1KtH5atZQUs9Achbce6LiOaJxLbksNJenvgvyKLsJrkc/edit#heading=h.ioikt02qpy5f).

Depends on:

- [Implement ability for extensions to be notified about effective
configuration opentelemetry-collector#6596](open-telemetry/opentelemetry-collector#6596)
- [Make service.instance.id and other telemetry attributes available to
extensions opentelemetry-collector#6599](open-telemetry/opentelemetry-collector#6599)

Closes
open-telemetry#16618

---------

Signed-off-by: Sean Porter <portertech@gmail.com>
Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this issue Nov 12, 2023
See [design
document](https://docs.google.com/document/d/1KtH5atZQUs9Achbce6LiOaJxLbksNJenvgvyKLsJrkc/edit#heading=h.ioikt02qpy5f).

Depends on:

- [Implement ability for extensions to be notified about effective
configuration opentelemetry-collector#6596](open-telemetry/opentelemetry-collector#6596)
- [Make service.instance.id and other telemetry attributes available to
extensions opentelemetry-collector#6599](open-telemetry/opentelemetry-collector#6599)

Closes
open-telemetry#16618

---------

Signed-off-by: Sean Porter <portertech@gmail.com>
Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
RoryCrispin pushed a commit to ClickHouse/opentelemetry-collector-contrib that referenced this issue Nov 24, 2023
See [design
document](https://docs.google.com/document/d/1KtH5atZQUs9Achbce6LiOaJxLbksNJenvgvyKLsJrkc/edit#heading=h.ioikt02qpy5f).

Depends on:

- [Implement ability for extensions to be notified about effective
configuration opentelemetry-collector#6596](open-telemetry/opentelemetry-collector#6596)
- [Make service.instance.id and other telemetry attributes available to
extensions opentelemetry-collector#6599](open-telemetry/opentelemetry-collector#6599)

Closes
open-telemetry#16618

---------

Signed-off-by: Sean Porter <portertech@gmail.com>
Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants