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.

Sign up for GitHub

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

Jump to bottom

Implement ability for extensions to be notified about effective configuration #6596

Closed
tigrannajaryan opened this issue Nov 21, 2022 · 4 comments · Fixed by #6833
Closed

Implement ability for extensions to be notified about effective configuration #6596

tigrannajaryan opened this issue Nov 21, 2022 · 4 comments · Fixed by #6833
Assignees
@evan-bradley
Labels
enhancement New feature or request
Milestone
OpAMP

Comments

@tigrannajaryan
Copy link
Member

tigrannajaryan commented Nov 21, 2022 •
edited
Loading

To implement OpAMP for the Collector we need a way for extensions to know what is the current effective configuration of the Collector, see the [design document](https://docs.google.com/document/d/1KtH5atZQUs9Achbce6LiOaJxLbksNJenvgvyKLsJrkc/edit#heading=h.ioikt02qpy5f.

This can be implemented as an optional interface that extensions can implement to be notified of the current config.

The config must be reported immediately after the extensions are started and then every time the config changes as a result of ConfigProvider reloads.
@tigrannajaryan tigrannajaryan added this to the OpAMP milestone Nov 21, 2022
@tigrannajaryan tigrannajaryan mentioned this issue Nov 23, 2022
Closed
5 tasks
@evan-bradley
Copy link
Contributor

I can work on this.

@tigrannajaryan
Copy link
Member Author

@evan-bradley feel free to add me a as a reviewer on all OpAMP-related PRs.

@jpkrohling jpkrohling added the enhancement New feature or request label Nov 29, 2022
@portertech portertech mentioned this issue Dec 2, 2022
Merged
@evan-bradley evan-bradley mentioned this issue Dec 19, 2022
Merged
@bogdandrutu
Copy link
Member

This can be implemented as an optional interface that extensions can implement to be notified of the current config.

Why not an optional interface on the Host? I would like to determine when to do one or the other.

@evan-bradley
Copy link
Contributor

Why not an optional interface on the Host? I would like to determine when to do one or the other.

I would like to use a push-based model for this so in the future the Host can update extensions that persist across configuration updates. Even if that doesn't happen anytime soon, having the Host push updates to extensions is not much more complex than having extensions call a method on the Host to get the effective configuration.

To notify extensions, we will either need to provide an interface on the Host that allows extensions to register message handlers, or allow the Host to call a method on extensions that want to be notified of updates. I think having an optional interface for extensions will provide the simplest solution. This also matches the approach to host-component communication for component status reporting in #6560.

@gbbr gbbr mentioned this issue Feb 21, 2023
Closed
dmitryax pushed a commit that referenced this issue Aug 8, 2023
@evan-bradley
Notify extensions of the Collector's effective configuration (#6833)
49a090b 
**Description:**

This adds an optional `ConfigWatcher` interface that extensions can
implement if they want to be notified of the effective configuration
that is used by the Collector.

I don't feel very strongly about any of the decisions I made in this PR,
so I am open to input if we would like to take a different approach
anywhere. I will leave some comments to explain the decisions I made.

**Link to tracking Issue:**
Closes
#6596

**Testing:**

I've made minimal unit test changes, but I expect to write more tests
once there is consensus on the direction for implementing this
functionality. I have done some manual testing to show that an extension
can get a YAML representation of the effective config using two YAML
input files.

---------

Co-authored-by: Evan Bradley <evan-bradley@users.noreply.github.com>
evan-bradley pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this issue Oct 26, 2023
@portertech @djaglowski @tigrannajaryan
OpAMP Agent Extension (#16594)
8a4c306 
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
@portertech @djaglowski
@tigrannajaryan @sigilioso
OpAMP Agent Extension (open-telemetry#16594)
ecab38e 
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
@portertech @djaglowski
@tigrannajaryan @jmsnll
OpAMP Agent Extension (open-telemetry#16594)
93d3647 
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
@portertech @djaglowski
@tigrannajaryan @RoryCrispin
OpAMP Agent Extension (open-telemetry#16594)
4503d2d 
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
Assignees

@evan-bradley evan-bradley

Labels
enhancement New feature or request
Projects
None yet
Milestone
OpAMP
Development

Successfully merging a pull request may close this issue.

Notify extensions of the Collector's effective configuration evan-bradley/opentelemetry-collector
4 participants
@jpkrohling @bogdandrutu @tigrannajaryan @evan-bradley