Skip to content

Conversation

RassK
Copy link
Contributor

@RassK RassK commented Sep 9, 2025

Changes

Adds heartbeats to support http transport scenario.

There is no client side api yet to provide info about components etc. Only reports static heartbeats.

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

@RassK RassK requested a review from a team as a code owner September 9, 2025 13:37
@github-actions github-actions bot added the comp:opamp.client Things related to OpenTelemetry.OpAmp.Client label Sep 9, 2025
Copy link

codecov bot commented Sep 9, 2025

Codecov Report

❌ Patch coverage is 57.25806% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.02%. Comparing base (efa16d9) to head (f5637ed).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...OpenTelemetry.OpAmp.Client/Internal/OpAmpClient.cs 0.00% 14 Missing ⚠️
...penTelemetry.OpAmp.Client/Internal/FrameBuilder.cs 48.00% 13 Missing ⚠️
...nt/Internal/Services/Heartbeat/HeartbeatService.cs 77.55% 11 Missing ⚠️
...ternal/Services/Heartbeat/ComponentHealthStatus.cs 0.00% 9 Missing ⚠️
...Telemetry.OpAmp.Client/Internal/FrameDispatcher.cs 72.22% 5 Missing ⚠️
...pAmp.Client/Internal/Settings/HeartbeatSettings.cs 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3095      +/-   ##
==========================================
+ Coverage   69.82%   70.02%   +0.20%     
==========================================
  Files         426      420       -6     
  Lines       16495    16559      +64     
==========================================
+ Hits        11517    11595      +78     
+ Misses       4978     4964      -14     
Flag Coverage Δ
unittests-Instrumentation.Cassandra ?
unittests-OpAmp.Client 70.35% <57.25%> (-3.88%) ⬇️

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

Files with missing lines Coverage Δ
...Client/Internal/Services/Heartbeat/HealthReport.cs 100.00% <100.00%> (ø)
...mp.Client/Internal/Settings/OpAmpClientSettings.cs 83.33% <100.00%> (ø)
...pAmp.Client/Internal/Settings/HeartbeatSettings.cs 50.00% <50.00%> (ø)
...Telemetry.OpAmp.Client/Internal/FrameDispatcher.cs 80.00% <72.22%> (+5.00%) ⬆️
...ternal/Services/Heartbeat/ComponentHealthStatus.cs 0.00% <0.00%> (ø)
...nt/Internal/Services/Heartbeat/HeartbeatService.cs 77.55% <77.55%> (ø)
...penTelemetry.OpAmp.Client/Internal/FrameBuilder.cs 64.44% <48.00%> (-20.56%) ⬇️
...OpenTelemetry.OpAmp.Client/Internal/OpAmpClient.cs 0.00% <0.00%> (ø)

... and 21 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.

}
catch (Exception ex)
{
// TODO: change to proper logging
Copy link
Member

Choose a reason for hiding this comment

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

Intentionally left for follow ups?

For now, all contrib packages are using EventSources similar to https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/8452d88e83e1c204bd4b43c7c3d8189f94ff87be/src/OpenTelemetry.Resources.Host/HostResourceEventSource.cs

(There are even tests for it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that was my plan as well. Will address logging in a separate PR.

@Kielek Kielek mentioned this pull request Sep 10, 2025
1 task
@Kielek Kielek merged commit 22aada2 into open-telemetry:main Sep 10, 2025
65 checks passed
@RassK RassK deleted the opamp-client-heartbeats branch September 10, 2025 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:opamp.client Things related to OpenTelemetry.OpAmp.Client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants