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

Exponential Bucket Histogram - part 1 #3462

Merged
merged 9 commits into from
Jul 20, 2022
Merged

Conversation

reyang
Copy link
Member

@reyang reyang commented Jul 20, 2022

This PR covers the implementation of base-2 exponential histogram (including the bucket inclusivity adjustment proposal from this PR) bucket lookup algorithm based on IEEE 754 float64 optimization.

I plan to cover exponential bucket histogram in a series of PRs:

  1. IEEE 754 float 64 bucket lookup algorithm and tests (focusing on .NET6 for now, everything will be internal only - no public API surface change) Exponential Bucket Histogram - part 1 #3462
  2. Automatic dynamic range adjustment and the ability to merge exponential histograms Exponential Bucket Histogram - part 2 #3468 Exponential Bucket Histogram - part 3 #3473 Exponential Bucket Histogram - part 4 #3477 Exponential Bucket Histogram - part 5 #3482 Exponential Bucket Histogram - part 6 #3494 Exponential Bucket Histogram - part 7 #3499 Exponential Bucket Histogram - part 8 #3505 Exponential Bucket Histogram - part 9 #3553
  3. Console exporter support
  4. In-memory exporter support
  5. Plumbing work with existing OpenTelemetry SDKs (e.g. configuration)
  6. Additional optimizations and corner case handling (e.g. lookup table which might require finding a perfect hash, the TODO items in step 1)
  7. Benchmarks and stress tests, potentially port the code to runtimes < net6.0 if there is a need

Future work that I haven't planned for at this moment:

  1. Integration with Prometheus Exporter
  2. Integration with OTLP Exporter

@reyang reyang requested a review from a team July 20, 2022 16:20
@codecov
Copy link

codecov bot commented Jul 20, 2022

Codecov Report

Merging #3462 (2376fb5) into main (3d6be9c) will increase coverage by 0.24%.
The diff coverage is 74.19%.

❗ Current head 2376fb5 differs from pull request most recent head 00f2233. Consider uploading reports for the commit 00f2233 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3462      +/-   ##
==========================================
+ Coverage   86.14%   86.39%   +0.24%     
==========================================
  Files         265      267       +2     
  Lines        9602     9633      +31     
==========================================
+ Hits         8272     8322      +50     
+ Misses       1330     1311      -19     
Impacted Files Coverage Δ
...Metrics/ExponentialBucketHistogramConfiguration.cs 0.00% <0.00%> (ø)
...penTelemetry/Metrics/ExponentialBucketHistogram.cs 76.66% <76.66%> (ø)
...Telemetry/Internal/SelfDiagnosticsEventListener.cs 97.65% <0.00%> (+0.78%) ⬆️
src/OpenTelemetry/Logs/OpenTelemetryLogger.cs 91.11% <0.00%> (+2.22%) ⬆️
...heus/Implementation/PrometheusCollectionManager.cs 82.27% <0.00%> (+2.53%) ⬆️
...metryProtocol/Implementation/ActivityExtensions.cs 94.50% <0.00%> (+3.29%) ⬆️
...tation/OpenTelemetryProtocolExporterEventSource.cs 85.00% <0.00%> (+10.00%) ⬆️
...xporter.OpenTelemetryProtocol/OtlpTraceExporter.cs 77.27% <0.00%> (+40.90%) ⬆️
...entation/ExportClient/OtlpGrpcTraceExportClient.cs 78.57% <0.00%> (+42.85%) ⬆️

@alanwest
Copy link
Member

alanwest commented Jul 20, 2022

Very much not to do with this PR, but this test has failed two times in a row... looking more closely at it

https://github.com/open-telemetry/opentelemetry-dotnet/runs/7433955618

Failed OpenTelemetry.Exporter.ZPages.Tests.ZPagesExporterTests.CheckingZPagesProcessor [58 ms]
  Error Message:
   System.Net.Http.HttpRequestException : Connection refused (localhost:7284)
---- System.Net.Sockets.SocketException : Connection refused
  Stack Trace:
     at System.Net.Http.HttpConnectionPool.ConnectToTcpHostAsync(String host, Int32 port, HttpRequestMessage initialRequest, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.ConnectAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.CreateHttp11ConnectionAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.AddHttp11ConnectionAsync(HttpRequestMessage request)
   at System.Threading.Tasks.TaskCompletionSourceWithCancellation`1.WaitWithCancellationAsync(CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.GetHttp11ConnectionAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.SendWithVersionDetectionAndRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken)
   at System.Net.Http.DiagnosticsHandler.SendAsyncCore(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
   at OpenTelemetry.Exporter.ZPages.Tests.ZPagesExporterTests.CheckingZPagesProcessor() in /home/runner/work/opentelemetry-dotnet/opentelemetry-dotnet/test/OpenTelemetry.Exporter.ZPages.Tests/ZPagesExporterTests.cs:line 1[67](https://github.com/open-telemetry/opentelemetry-dotnet/runs/7433955618?check_suite_focus=true#step:5:68)
--- End of stack trace from previous location ---
----- Inner Stack Trace -----
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.ThrowException(SocketError error, CancellationToken cancellationToken)
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.System.Threading.Tasks.Sources.IValueTaskSource.GetResult(Int16 token)
   at System.Net.Sockets.Socket.<ConnectAsync>g__WaitForConnectWithCancellation|277_0(AwaitableSocketAsyncEventArgs saea, ValueTask connectTask, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.ConnectToTcpHostAsync(String host, Int32 port, HttpRequestMessage initialRequest, Boolean async, CancellationToken cancellationToken)

I think #3463 will fix it.

@reyang reyang added pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package metrics Metrics signal related labels Jul 20, 2022
@utpilla utpilla merged commit 95419ed into open-telemetry:main Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
metrics Metrics signal related pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants