Unify addons and agent packages #77
Conversation
|
@open-telemetry/opamp-spec-approvers I made this a draft for now, please tell what you think about it. |
|
@andykellr it would be also good to have your thoughts on this. |
|
I can look at it in more detail tomorrow or Wednesday, but at first glance it makes sense to me. |
tigrannajaryan
force-pushed
the
feature/unify-addons-and-packages
branch
2 times, most recently
from
1291413
to
ff53ff1
Compare
The action is not properly triggered on PRs, e.g. open-telemetry#77 I am not completely sure why, but I copied the "on" trigger from the https://github.com/open-telemetry/opentelemetry-specification repo where the action works fine, so hopefully this should fix it.
The action is not properly triggered on PRs, e.g. #77 I am not completely sure why, but I copied the "on" trigger from the https://github.com/open-telemetry/opentelemetry-specification repo where the action works fine, so hopefully this should fix it.
Addons and agent packages were previously defined separately in the spec but had a lot of similarities. This changes combines them into one concept of "packages". The resulting spec is significantly shorter while providing the same functionality. It may also reduce the code duplication in implementations. Resolves open-telemetry#4
tigrannajaryan
force-pushed
the
feature/unify-addons-and-packages
branch
from
ff53ff1
to
4c5d4d8
Compare
|
@open-telemetry/opamp-spec-approvers I spent a bit more time thinking about this change and feel reasonably confident that this is a move in the right direction. I think this is ready for review. |
specification.md
Outdated
| TODO: is there a need to prescribe any specific behavior or explain what happens if there | ||
| are more than one top-level packages in the server's offer or we can leave it out and | ||
| allow agent-specific behavior? | ||
|
|
There was a problem hiding this comment.
I think this can be agent specific.
Related to this, I think a name string could be useful on the PackageAvailable or DownloadableFile message identifying the package. This may not necessarily be determined from the download_url.
There was a problem hiding this comment.
I think this can be agent specific.
I agree, I thought the same way. I will delete the TODO.
Related to this, I think a name string could be useful on the PackageAvailable or DownloadableFile message identifying the package. This may not necessarily be determined from the download_url.
PackageAvailable is only referenced from packages map in PackagesAvailable field, where the keys of the map are the package names. Adding the name to PackageAvailable would be a duplicate. Or are you thinking about something else?
For DownloadableFile: are you looking for the package name or the file name here?
There was a problem hiding this comment.
That makes sense. I think the key in the map is fine.
|
@pmm-sumo it would be great if you could also have a look at this. |
Applies spec change open-telemetry/opamp-spec#77
Applies spec change open-telemetry/opamp-spec#77
Applies spec change open-telemetry/opamp-spec#77
Applies spec change open-telemetry/opamp-spec#77
Applies spec change open-telemetry/opamp-spec#77
Applies spec change open-telemetry/opamp-spec#77
Addons and agent packages were previously defined separately
in the spec but had a lot of similarities. This change combines
them into one concept of "packages". The resulting spec is
significantly shorter while providing the same functionality.
It may also reduce the code duplication in implementations.
Resolves #4