Add a basic http OpAMP client#3635
Conversation
opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/agent.py
Outdated
Show resolved
Hide resolved
|
@open-telemetry/opamp-spec-approvers could you help review this? thanks! |
3e67115 to
e5fced7
Compare
opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/transport/requests.py
Show resolved
Hide resolved
opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/client.py
Outdated
Show resolved
Hide resolved
1d2061f to
04d8923
Compare
4cfbf33 to
a746292
Compare
opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/version.py
Outdated
Show resolved
Hide resolved
opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/client.py
Outdated
Show resolved
Hide resolved
1d85953 to
cf479f2
Compare
|
I can't unresolve threads, but I've replied above. |
|
Thank you for working on this.
One thing that would be great to add is interoperability tests between Go and Python implementations. Ideally we would have both pairs (Client in Python, Server in Go and the vice versa) connecting and performing OpAMP exchanges to make sure the pair works correctly together. OpAMP Go implementation presumably is the most complete implementation at the moment, so all other languages (including Python) could use it as a reference implementation to test against. I think we can spend a bit time adding any necessary tooling to opamp-go that makes this possible (mock servers, clients that probe capabilities, etc). Unfortunately I don't have time myself but if anyone wants to work on the design and implementation I can dedicate some time to review the design. |
This PR has some recorded e2e tests doing this, we don't test many scenarios but at least we test against a real response.
That would be helpful indeed |
b48e242 to
3eb1769
Compare
aabmass
left a comment
There was a problem hiding this comment.
Giving this an approving "python review" but I don't know much about OpAmp. Looks good enough to merge though as an initial prototype with proper disclaimers.
🚢
opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/agent.py
Outdated
Show resolved
Hide resolved
opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/agent.py
Outdated
Show resolved
Hide resolved
opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/agent.py
Outdated
Show resolved
Hide resolved
a483c24 to
6185955
Compare
.../tests/opamp/cassettes/test_connection_remote_config_status_heartbeat_disconnection.yaml.lel
Outdated
Show resolved
Hide resolved
Introducing basic handling of the ReportsEffectiveConfig capability
6185955 to
aa7b346
Compare
|
Hi @xrmx, thanks so much for creating this! I’ve been using the client for a PoC and had a few questions as I was going through the changes.
Totally understand if these are out of scope for this PR, wanted to ask in case there's been any discussion around them. I’m also happy to move this discussion to a new issue and take on the implementation work if that would be helpful. Thanks again! |
opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/agent.py
Outdated
Show resolved
Hide resolved
I think can change the interface to match theirs, now you have just one callback and you have to do that on your own
This stuff is not specified so I've implemented only what we are using :) Of course we can generalize that. Maybe some specs will come from open-telemetry/opentelemetry-specification#4738
Wasn't aware of that, I'll need to check that
May be out of scope for this PR but perfectly fine to discuss. Thanks! In the last push I've made this package released on its own so we can iterate on this without interfering with the releases. |
pmcollins
left a comment
There was a problem hiding this comment.
Ran changes locally and LGTM. Thanks for doing this.
* Add a basic http OpAMP client * Add some docs and hook it into the system Still not building content * Add default value of 30 seconds to heartbeat message interval * Fix docs build * More docs improvements * Fix spellcheck * Remove local workaround * Generate workflows and add to release script * Fix typos in opamp lint commands * Fix requirements for pylint * Update opamp/opentelemetry-opamp-client/pyproject.toml * Recreate requirements * Add missing opentelemetry-api dependency * Fix tox test commands Drop opentelemetry api fixed version from requirements * Fix tox * Add baseline of vcrpy 7.0.0 * Ignore pb2 module in pylintrc * Bump pylint to match the version in core * Silence pylint warnings * Don't trace opamp client own http requests * Permit to pass a custom transport to client And a custom session to RequestsTransport * Don't bump pylint after all * Fix pylint * Try to typecheck opamp client * Bump version after rebase * Fix typecheck in client * Please pyright in strict mode * No need for functions and methods to be private since _opamp module is already private * Add missing protobuf package installation for typecheck * Fix docs generation * Fix pyright exclusion rule for proto Missed .pyi exclusion * Feedback * Don't flush the queue at exit * Log transport send exceptions * Update example to not assume that the config is in json format * Fix typo in exception * Looks like it's implementers * Add timeout to stop to forward to threads join * Clarify doc * Fix typo in var name * Add support for mTLS * Add helpers for handling of ReportFullState ServerToAgent flag Introducing basic handling of the ReportsEffectiveConfig capability * Remove backup file * Rewrite opamp_proto_codegen.sh to use uv * Make the package releasable independently * Send full state at connection * Add 3.14 test run * Add changelog entry * Add missing ReportsEffectiveConfig capability in documentation * Start version from 0.1b0 and re-record e2e tests * Record tests against opentelemetry-go --------- Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com>
Description
This introduces a basic OpAMP http client for handling remote configuration. The client implements a bunch of capabilities (ReportsStatus, ReportsHeartbeat, AcceptsRemoteConfig, ReportsRemoteConfig) that are enough to get a remote config from an opamp server, parse it, apply it and ack it. Since OTel / OpAMP do not standardize APIs, config options or environment variables the distros are required to provide code doing so.
OTel Python distros would need to provide their own message handler callback that implements the actual change of whatever configuration their backends sends.
In practice distro would need to do something like the following:
The module is called _opamp because it's a bit early to standardize on an api. The code is divided roughly in:
OpAMP reference: https://opentelemetry.io/docs/specs/opamp/.
This is tested against https://github.com/elastic/opentelemetry-collector-components/ that is using the opamp-go implementation.
TODO:
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.