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

AgentIdentification protobuf and callback #58

Merged
merged 3 commits into from
Mar 29, 2022

Conversation

pmm-sumo
Copy link
Contributor

Implements OpAMP Spec change open-telemetry/opamp-spec#63

@pmm-sumo pmm-sumo requested a review from a team March 22, 2022 19:26
@pmm-sumo
Copy link
Contributor Author

pmm-sumo commented Mar 22, 2022

@tigrannajaryan do you think it would be worthwhile to implement this in the example agent/server? I was wondering about that but then wasn't fully sure if it wouldn't make things too complex for the sake of the examples (e.g. agent would need to persist the instance id)

@tigrannajaryan
Copy link
Member

@tigrannajaryan do you think it would be worthwhile to implement this in the example agent/server? I was wondering about that but then wasn't fully sure if it wouldn't make things too complex for the sake of the examples (e.g. agent would need to persist the instance id)

I think it is worth showing in the example. Doesn't necessarily have to persist the instance id, can just keep it in memory during the execution session.

client/callbacks.go Outdated Show resolved Hide resolved
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM now, just a couple small comments.

client/internal/receiver.go Outdated Show resolved Hide resolved
client/internal/receiver.go Outdated Show resolved Hide resolved
@tigrannajaryan
Copy link
Member

Also needs a rebase.

pmm-sumo and others added 2 commits March 29, 2022 17:26
Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
@tigrannajaryan tigrannajaryan merged commit c148760 into open-telemetry:main Mar 29, 2022
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

Successfully merging this pull request may close these issues.

2 participants