-
Notifications
You must be signed in to change notification settings - Fork 402
[OpAMP] Expose public RemoteConfigMessage
#3614
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
Merged
Kielek
merged 5 commits into
open-telemetry:main
from
stevejgordon:opamp-expose-messages
Dec 16, 2025
Merged
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
9380af2
Proof of concept for public message types
stevejgordon f9f79c3
Refine public API for remote config message
stevejgordon 30b794c
Support config hash, PR feedback and add tests
stevejgordon d42f72f
Update changelog
stevejgordon 727de3e
Simplify the public API and update tests
stevejgordon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
12 changes: 12 additions & 0 deletions
12
src/OpenTelemetry.OpAmp.Client/.publicApi/PublicAPI.Unshipped.txt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 0 additions & 17 deletions
17
src/OpenTelemetry.OpAmp.Client/Internal/Messages/RemoteConfigMessage.cs
This file was deleted.
Oops, something went wrong.
70 changes: 70 additions & 0 deletions
70
src/OpenTelemetry.OpAmp.Client/Messages/RemoteConfiguration/AgentConfigFile.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| using Google.Protobuf; | ||
|
|
||
| namespace OpenTelemetry.OpAmp.Client.Messages; | ||
|
|
||
| /// <summary> | ||
| /// Represents an agent configuration file. | ||
| /// </summary> | ||
| public class AgentConfigFile | ||
| { | ||
| private readonly ByteString body; | ||
|
|
||
| internal AgentConfigFile(string name, global::OpAmp.Proto.V1.AgentConfigFile agentConfigFile) | ||
| { | ||
| this.body = agentConfigFile.Body ?? ByteString.Empty; | ||
| this.ContentType = agentConfigFile.ContentType; | ||
| this.Name = name; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the length, in bytes, of the configuration file body. | ||
| /// </summary> | ||
| public int BodyLength => this.body.Length; | ||
|
|
||
| /// <summary> | ||
| /// Gets the MIME Content-Type that describes the data contained in the body of the remote configuration file. | ||
| /// </summary> | ||
| public string? ContentType { get; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the name of this configuration file. | ||
| /// </summary> | ||
| public string Name { get; } | ||
|
|
||
| /// <summary> | ||
| /// Returns the configuration file body as a byte array. | ||
| /// </summary> | ||
| /// <returns>A byte array containing the contents of the message body. The array is empty if the body has no content.</returns> | ||
| public byte[] GetBodyBytes() => this.body.ToByteArray() ?? []; | ||
|
|
||
| /// <summary> | ||
| /// Attempts to copy the configuration file body to the specified destination buffer. | ||
| /// </summary> | ||
| /// <remarks>If the body is empty, no data is written and the method returns <c>true</c> with <paramref name="bytesWritten"/> set to | ||
| /// 0. If the destination buffer is too small to hold the body content, no data is written, <paramref name="bytesWritten"/> is set to | ||
| /// 0, and the method returns <c>false</c>.</remarks> | ||
| /// <param name="destination">The buffer that receives the body bytes. Must be large enough to hold the entire body content.</param> | ||
| /// <param name="bytesWritten">When this method returns, contains the number of bytes successfully written to the destination buffer.</param> | ||
| /// <returns><c>true</c> if the body was successfully copied to the destination buffer or if the body is empty; otherwise, <c>false</c>.</returns> | ||
| public bool TryGetBody(Span<byte> destination, out int bytesWritten) | ||
| { | ||
| if (this.body.IsEmpty) | ||
| { | ||
| bytesWritten = 0; | ||
| return true; | ||
| } | ||
|
|
||
| if (destination.Length < this.body.Length) | ||
| { | ||
| bytesWritten = 0; | ||
| return false; | ||
| } | ||
|
|
||
| this.body.Span.CopyTo(destination); | ||
| bytesWritten = this.body.Length; | ||
| return true; | ||
| } | ||
| } | ||
87 changes: 87 additions & 0 deletions
87
src/OpenTelemetry.OpAmp.Client/Messages/RemoteConfiguration/RemoteConfigMessage.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| using Google.Protobuf; | ||
| using OpAmp.Proto.V1; | ||
|
|
||
| namespace OpenTelemetry.OpAmp.Client.Messages; | ||
|
|
||
| /// <summary> | ||
| /// Represents an OpAMP server-to-agent remote configuration message. | ||
| /// </summary> | ||
| public class RemoteConfigMessage : OpAmpMessage | ||
| { | ||
| private readonly Dictionary<string, AgentConfigFile> agentConfigMap; | ||
| private readonly ByteString configHash; | ||
|
|
||
| internal RemoteConfigMessage(AgentRemoteConfig agentRemoteConfig) | ||
| { | ||
| this.agentConfigMap = new Dictionary<string, AgentConfigFile>(agentRemoteConfig.Config.ConfigMap.Count, StringComparer.Ordinal); | ||
|
|
||
| foreach (var config in agentRemoteConfig.Config.ConfigMap) | ||
| { | ||
| if (config.Value is not null) | ||
| { | ||
| this.agentConfigMap[config.Key] = new AgentConfigFile(config.Key, config.Value); | ||
| } | ||
| } | ||
|
|
||
| this.configHash = agentRemoteConfig.ConfigHash; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets a dictionary of agent configuration files, keyed by the name of the configuration file. | ||
| /// </summary> | ||
| public IReadOnlyDictionary<string, AgentConfigFile> AgentConfigMap => this.agentConfigMap; | ||
|
|
||
| /// <summary> | ||
| /// Gets the length, in bytes, of the configuration hash. | ||
| /// </summary> | ||
| public int HashLength => this.configHash.Length; | ||
|
|
||
| /// <summary> | ||
| /// Returns the configuration hash as a byte array. | ||
| /// </summary> | ||
| /// <returns>A <see cref="byte"/> array containing the hash of the remote configuration.</returns> | ||
| public byte[] GetConfigHashBytes() => this.configHash.ToByteArray(); | ||
|
|
||
| /// <summary> | ||
| /// Returns the configuration hash as a UTF-8 string. | ||
| /// </summary> | ||
| /// <returns>A <see cref="string"/> representing the UTF-8 encoded configuration hash.</returns> | ||
| public string GetConfigHashUtf8String() => this.configHash.ToStringUtf8(); | ||
|
|
||
| /// <summary> | ||
| /// Attempts to copy the configuration hash to the specified destination buffer. | ||
| /// </summary> | ||
| /// <remarks>If the hash is empty, no data is written and the method returns <c>true</c> with <paramref name="bytesWritten"/> set to | ||
| /// 0. If the destination buffer is too small to hold the hash, no data is written, <paramref name="bytesWritten"/> is set to | ||
| /// 0, and the method returns <c>false</c>.</remarks> | ||
| /// <param name="destination">The buffer that receives the hash bytes. Must be large enough to hold the entire hash content.</param> | ||
| /// <param name="bytesWritten">When this method returns, contains the number of bytes successfully written to the destination buffer.</param> | ||
| /// <returns><c>true</c> if the hash was successfully copied to the destination buffer or if the hash is empty; otherwise, <c>false</c>.</returns> | ||
| public bool TryGetConfigHash(Span<byte> destination, out int bytesWritten) | ||
| { | ||
| if (this.configHash is null) | ||
| { | ||
| bytesWritten = 0; | ||
| return false; | ||
| } | ||
|
|
||
| if (this.configHash.IsEmpty) | ||
| { | ||
| bytesWritten = 0; | ||
| return true; | ||
| } | ||
|
|
||
| if (destination.Length < this.configHash.Length) | ||
| { | ||
| bytesWritten = 0; | ||
| return false; | ||
| } | ||
|
|
||
| this.configHash.Span.CopyTo(destination); | ||
| bytesWritten = this.configHash.Length; | ||
| return true; | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if
byte[]?would be better to be able to distinguish between "not set" and "genuinely empty" (assuming such a distinction is meaningful here).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's unclear to me from the spec if this could ever be null. In theory, not as it should only send the config if there's something to send. My gut is that the expected scenario is there is always some data so avoiding making this nullable and requiring the user to null check is slightly more pleasant. The spec specifically calls out that the key may be empty, but doesn't specify scenarios were the config could be null.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One topic to the public contract, I have missed in previous PR.
Shouldn't
ReadOnlySpan<byte>be a better option here? the body, exposes Span property. We could avoid allocations.It is related to other places in the public API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kielek We did have that exposed at one point but I think at the time I'd missed that
ByteStringhas a span directly accessible. I'll update as we can get rid of the methods and TryGet pattern, just relying on exposing the Span for consumers to do whatever they need with.