[OpAMP.Client] Add support for plain http transport #2926

RassK · 2025-07-18T12:47:32Z

Changes

Adds support for plain http transport (send & receive).

Merge requirement checklist

CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
Unit tests added/updated
Appropriate CHANGELOG.md files updated for non-trivial changes
Changes in public API reviewed (if applicable)

Signed-off-by: RassK <[email protected]>

RassK · 2025-07-18T12:49:33Z

src/OpenTelemetry.OpAmp.Client/Protos/opamp.proto

+import "anyvalue.proto";
+
+option go_package = "github.com/open-telemetry/opamp-go/protobufs";
+option csharp_namespace = "OpAmp.Protocol";


Still under review open-telemetry/opamp-spec#243

codecov · 2025-07-18T13:04:23Z

Codecov Report

❌ Patch coverage is 61.08108% with 72 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.58%. Comparing base (d97922c) to head (54d57a8).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/OpenTelemetry.OpAmp.Client/OpAmpClient.cs	0.00%	12 Missing ⚠️
src/OpenTelemetry.OpAmp.Client/FrameProcessor.cs	82.00%	9 Missing ⚠️
src/OpenTelemetry.OpAmp.Client/FrameDispatcher.cs	75.00%	6 Missing ⚠️
...t/Listeners/Messages/AgentIdentificationMessage.cs	0.00%	4 Missing ⚠️
...p.Client/Listeners/Messages/CapabilitiesMessage.cs	0.00%	4 Missing ⚠️
....OpAmp.Client/Listeners/Messages/CommandMessage.cs	0.00%	4 Missing ⚠️
...nt/Listeners/Messages/ConnectionSettingsMessage.cs	0.00%	4 Missing ⚠️
...nt/Listeners/Messages/CustomCapabilitiesMessage.cs	0.00%	4 Missing ⚠️
....Client/Listeners/Messages/ErrorResponseMessage.cs	0.00%	4 Missing ⚠️
...ry.OpAmp.Client/Listeners/Messages/FlagsMessage.cs	0.00%	4 Missing ⚠️
... and 6 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2926      +/-   ##
==========================================
- Coverage   69.79%   69.58%   -0.22%     
==========================================
  Files         396      413      +17     
  Lines       15992    16177     +185     
==========================================
+ Hits        11161    11256      +95     
- Misses       4831     4921      +90

Flag	Coverage Δ
unittests-Contrib.Shared.Tests	`86.25% <ø> (ø)`
unittests-Exporter.Geneva	`53.33% <ø> (-0.34%)`	⬇️
unittests-Exporter.InfluxDB	`95.14% <ø> (ø)`
unittests-Exporter.Instana	`74.86% <ø> (ø)`
unittests-Exporter.OneCollector	`94.61% <ø> (ø)`
unittests-Extensions	`90.65% <ø> (ø)`
unittests-Extensions.Enrichment	`100.00% <ø> (ø)`
unittests-Instrumentation.AWS	`83.69% <ø> (ø)`
unittests-Instrumentation.AspNet	`74.93% <ø> (ø)`
unittests-Instrumentation.AspNetCore	`70.32% <ø> (ø)`
unittests-Instrumentation.Cassandra	`23.52% <ø> (ø)`
unittests-Instrumentation.ConfluentKafka	`14.10% <ø> (ø)`
unittests-Instrumentation.ElasticsearchClient	`80.12% <ø> (ø)`
unittests-Instrumentation.EntityFrameworkCore	`78.63% <ø> (ø)`
unittests-Instrumentation.EventCounters	`76.36% <ø> (ø)`
unittests-Instrumentation.GrpcCore	`91.42% <ø> (ø)`
unittests-Instrumentation.GrpcNetClient	`79.61% <ø> (ø)`
unittests-Instrumentation.Hangfire	`84.61% <ø> (ø)`
unittests-Instrumentation.Http	`74.09% <ø> (ø)`
unittests-Instrumentation.Owin	`88.62% <ø> (ø)`
unittests-Instrumentation.Process	`100.00% <ø> (ø)`
unittests-Instrumentation.Quartz	`78.76% <ø> (ø)`
unittests-Instrumentation.Runtime	`100.00% <ø> (ø)`
unittests-Instrumentation.ServiceFabricRemoting	`34.54% <ø> (ø)`
unittests-Instrumentation.SqlClient	`88.50% <ø> (ø)`
unittests-Instrumentation.StackExchangeRedis	`70.81% <ø> (ø)`
unittests-Instrumentation.Wcf	`78.95% <ø> (ø)`
unittests-OpAmp.Client	`61.08% <61.08%> (?)`
unittests-PersistentStorage	`65.55% <ø> (-0.34%)`	⬇️
unittests-Resources.AWS	`75.00% <ø> (ø)`
unittests-Resources.Azure	`84.56% <ø> (ø)`
unittests-Resources.Container	`67.34% <ø> (ø)`
unittests-Resources.Gcp	`71.42% <ø> (ø)`
unittests-Resources.Host	`73.91% <ø> (ø)`
unittests-Resources.OperatingSystem	`76.98% <ø> (ø)`
unittests-Resources.Process	`100.00% <ø> (ø)`
unittests-Resources.ProcessRuntime	`79.59% <ø> (ø)`
unittests-Sampler.AWS	`88.25% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
....Client/Listeners/Messages/CustomMessageMessage.cs	`100.00% <100.00%> (ø)`
.../OpenTelemetry.OpAmp.Client/OpAmpClientSettings.cs	`80.00% <80.00%> (ø)`
...OpenTelemetry.OpAmp.Client/Utils/SequenceHelper.cs	`66.66% <66.66%> (ø)`
src/OpenTelemetry.OpAmp.Client/FrameBuilder.cs	`85.00% <85.00%> (ø)`
...t/Listeners/Messages/AgentIdentificationMessage.cs	`0.00% <0.00%> (ø)`
...p.Client/Listeners/Messages/CapabilitiesMessage.cs	`0.00% <0.00%> (ø)`
....OpAmp.Client/Listeners/Messages/CommandMessage.cs	`0.00% <0.00%> (ø)`
...nt/Listeners/Messages/ConnectionSettingsMessage.cs	`0.00% <0.00%> (ø)`
...nt/Listeners/Messages/CustomCapabilitiesMessage.cs	`0.00% <0.00%> (ø)`
....Client/Listeners/Messages/ErrorResponseMessage.cs	`0.00% <0.00%> (ø)`
... and 7 more

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

martincostello · 2025-07-18T13:00:34Z

src/OpenTelemetry.OpAmp.Client/CHANGELOG.md

-  `OpenTelemetry.OpAmp.Client`
-  project.
+* Initial release of `OpenTelemetry.OpAmp.Client` project.
+* Added support for OpAMP Plain Http transport.


Also needs a link to this PR.

Suggested change

* Added support for OpAMP Plain Http transport.

* Added support for OpAMP Plain HTTP transport.

martincostello · 2025-07-18T13:24:26Z

src/OpenTelemetry.OpAmp.Client/FrameBuilder.cs

+
+namespace OpenTelemetry.OpAmp.Client;
+
+internal class FrameBuilder : IFrameBuilder


Unless there's inheritance happening, internal classes can be sealed as this can help the JIT create better code.

src/OpenTelemetry.OpAmp.Client/FrameBuilder.cs

src/OpenTelemetry.OpAmp.Client/FrameProcessor.cs

src/OpenTelemetry.OpAmp.Client/Listeners/Messages/AgentIdentificationMessage.cs

martincostello · 2025-07-18T13:33:28Z

src/OpenTelemetry.OpAmp.Client/OpAmpClientSettings.cs

+/// <remarks>This enumeration defines the available transport protocols for communication. Use <see
+/// cref="WebSocket"/> for WebSocket-based communication, or <see cref="Http"/> for
+/// HTTP-based communication.</remarks>
+#pragma warning disable SA1201 // Elements should appear in the correct order


Fix rather than suppress?

I was mistakenly looking that SA1201 doesn't like it either way, basically prohibits multiple classes in the same file, but didn't want to extract it (yet, or at least not into root).

martincostello · 2025-07-18T13:35:05Z

src/OpenTelemetry.OpAmp.Client/OpenTelemetry.OpAmp.Client.csproj

+  <!-- .NET Framework supporting packages -->
+  <ItemGroup Condition="'$(TargetFramework)' == '$(NetFrameworkMinimumSupportedVersion)'">
+    <PackageReference Include="System.Net.Http" Version="4.3.4" />
+    <PackageReference Include="System.Collections.Immutable" Version="9.0.7" />


Considering open-telemetry/opentelemetry-dotnet#6327, I suggest starting with the lowest version possible (e.g. 8.0.0).

martincostello · 2025-07-18T13:37:16Z

src/OpenTelemetry.OpAmp.Client/Transport/Http/PlainHttpTransport.cs

+        if (!response.IsSuccessStatusCode)
+        {
+            throw new HttpRequestException($"Failed to send message: {response.ReasonPhrase}");
+        }


Just use EnsureSuccessStatusCode()?

martincostello · 2025-07-18T13:38:13Z

src/OpenTelemetry.OpAmp.Client/Transport/Http/PlainHttpTransport.cs

+    public void Dispose()
+    {
+        this.httpClient?.Dispose();
+        this.handler?.Dispose();
+    }


The HttpClient should be able to be constructed so that it can be told to dispose of the handler itself, then this instance doesn't need to do that or keep a reference to it.

Then you need to suppress CA2000 manually. disposeHandler is there but static analysis does not pass.

Signed-off-by: RassK <[email protected]>

RassK · 2025-07-18T19:51:12Z

FYI: PTO 🌴, Will be back in 4th of August. Feel free to push changes and merge if critical stuff got resolved.

(you can find context in #2872 if there is something looking very weird, there is kind of a big picture)

github-actions · 2025-08-02T03:39:02Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

…metry-dotnet-contrib into opamp-client-http

RassK · 2025-08-04T08:44:23Z

Q: are you tracking aot incompatible libs somewhere? - seems protobuf is not compatible, therefore opamp also.

martincostello · 2025-08-04T13:28:48Z

There's an issue in Google.Protobuf that broke some native AoT compatibility that I'm aware of: protocolbuffers/protobuf#21824

RassK · 2025-08-18T09:57:08Z

@martincostello do you see anything else that requires attention, since we wait for spec to commit changes.

src/OpenTelemetry.OpAmp.Client/FrameProcessor.cs

martincostello · 2025-08-18T12:04:03Z

src/OpenTelemetry.OpAmp.Client/OpAmpClientSettings.cs

+/// <remarks>This enumeration defines the available transport protocols for communication. Use <see
+/// cref="WebSocket"/> for WebSocket-based communication, or <see cref="Http"/> for
+/// HTTP-based communication.</remarks>
+internal enum ConnectionType


This should be in its own file.

where would you put it? ... I did not like root.

I would put it in ConnectionType.cs in the root, as otherwise I think the analyzers would complain about the style (having more than one type defined in the same file).

So far no errors like in #2926 (comment)

In the PoC branch I added them to Settings (since the subs would bloat root).
Those will become public and after that are not movable (not without creating a mess in namespaces).-
So need to get that internal / public boundary right.

src/OpenTelemetry.OpAmp.Client/OpAmpClientSettings.cs

.github/workflows/ci.yml

src/OpenTelemetry.OpAmp.Client/OpenTelemetry.OpAmp.Client.csproj

Co-authored-by: Martin Costello <[email protected]>

Kielek

LGTM. I will merge as is. It is good groundwork for further changes. I would like to avoid any more huge PRs.

Changes related to the proto namespace can be easily fixed later.

Kielek · 2025-08-19T12:30:04Z

src/OpenTelemetry.OpAmp.Client/OpenTelemetry.OpAmp.Client.csproj


+  <ItemGroup>
+    <PackageReference Include="Google.Protobuf" Version="3.31.1" />
+    <PackageReference Include="Grpc.Tools" Version="2.72.0">


Just a note: It will not work on ARM, if anybody decide to compile on this machine.

grpc/grpc#38538

Kielek · 2025-08-19T12:30:47Z

src/OpenTelemetry.OpAmp.Client/OpenTelemetry.OpAmp.Client.csproj


  <PropertyGroup>
-    <TargetFrameworks>$(NetMinimumSupportedVersion);$(NetStandardMinimumSupportedVersion);$(NetFrameworkMinimumSupportedVersion)</TargetFrameworks>
+    <TargetFrameworks>net9.0;$(NetMinimumSupportedVersion);$(NetStandardMinimumSupportedVersion);$(NetFrameworkMinimumSupportedVersion)</TargetFrameworks>


Note for myself, uid should be generated based on Guid.V7. It is available on .NET9+.

OpAmp plain http transport

b236c0b

Signed-off-by: RassK <[email protected]>

RassK requested a review from a team as a code owner July 18, 2025 12:47

github-actions bot assigned RassK Jul 18, 2025

github-actions bot added the comp:opamp.client Things related to OpenTelemetry.OpAmp.Client label Jul 18, 2025

RassK changed the title ~~[OpAmp.Client] Add support for plain http transport~~ [OpAMP.Client] Add support for plain http transport Jul 18, 2025

RassK commented Jul 18, 2025

View reviewed changes

RassK added 2 commits July 18, 2025 16:01

fix warnings

34fea2b

spelling fix

f1fe378

martincostello reviewed Jul 18, 2025

View reviewed changes

RassK added 2 commits July 18, 2025 22:39

feedback

0ce14b0

remove aot verification

f494074

Signed-off-by: RassK <[email protected]>

github-actions bot added the infra Infra work - CI/CD, code coverage, linters label Jul 18, 2025

thompson-tomo mentioned this pull request Jul 21, 2025

CONTRIBUTING - update PR with example of new project #2920

Merged

1 task

Merge branch 'main' into opamp-client-http

84bfe5e

github-actions bot added the Stale label Aug 2, 2025

RassK added 5 commits August 4, 2025 11:19

Update proto namespaces

9591147

Merge branch 'opamp-client-http' of https://github.com/RassK/opentele…

f96fec1

…metry-dotnet-contrib into opamp-client-http

Merge branch 'main' into opamp-client-http

886be93

fix test proto namespace

8db011f

remove aot test

559c42b

github-actions bot removed the Stale label Aug 5, 2025

RassK added 2 commits August 11, 2025 01:51

Merge branch 'main' into opamp-client-http

5c4ee57

Merge branch 'main' into opamp-client-http

dcc3b61

martincostello reviewed Aug 18, 2025

View reviewed changes

RassK added 2 commits August 18, 2025 15:42

seal classes

4c90ba3

add guid v7 support for .net9+

8506ef0

martincostello reviewed Aug 18, 2025

View reviewed changes

src/OpenTelemetry.OpAmp.Client/OpenTelemetry.OpAmp.Client.csproj Outdated Show resolved Hide resolved

Update src/OpenTelemetry.OpAmp.Client/OpenTelemetry.OpAmp.Client.csproj

bd930fa

Co-authored-by: Martin Costello <[email protected]>

martincostello approved these changes Aug 18, 2025

View reviewed changes

Merge branch 'main' into opamp-client-http

54d57a8

Kielek approved these changes Aug 19, 2025

View reviewed changes

Kielek merged commit da244a5 into open-telemetry:main Aug 19, 2025
223 checks passed

	* Added support for OpAMP Plain Http transport.
	* Added support for OpAMP Plain HTTP transport.


		namespace OpenTelemetry.OpAmp.Client;

		internal class FrameBuilder : IFrameBuilder

[OpAMP.Client] Add support for plain http transport #2926

[OpAMP.Client] Add support for plain http transport #2926

Uh oh!

Conversation

RassK commented Jul 18, 2025

Changes

Merge requirement checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RassK Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RassK commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 2, 2025

Uh oh!

RassK commented Aug 4, 2025

Uh oh!

martincostello commented Aug 4, 2025

Uh oh!

RassK commented Aug 18, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kielek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jul 18, 2025 •

edited

Loading

RassK Jul 18, 2025 •

edited

Loading

RassK commented Jul 18, 2025 •

edited

Loading