Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ require (
github.com/mattn/go-isatty v0.0.20
github.com/mdp/qrterminal/v3 v3.2.1
github.com/prometheus/client_golang v1.21.1
github.com/prometheus/client_model v0.6.1
github.com/prometheus/common v0.63.0
github.com/rs/zerolog v1.33.0
github.com/spf13/cast v1.7.1
Expand All @@ -53,6 +54,11 @@ require (
github.com/spf13/viper v1.20.0
github.com/stretchr/testify v1.10.0
github.com/tendermint/go-amino v0.16.0
go.opentelemetry.io/otel v1.35.0
go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.35.0
go.opentelemetry.io/otel/metric v1.35.0
go.opentelemetry.io/otel/sdk v1.35.0
go.opentelemetry.io/otel/sdk/metric v1.35.0
go.uber.org/mock v0.5.0
golang.org/x/crypto v0.36.0
golang.org/x/sync v0.12.0
Expand Down Expand Up @@ -101,6 +107,8 @@ require (
github.com/go-kit/kit v0.13.0 // indirect
github.com/go-kit/log v0.2.1 // indirect
github.com/go-logfmt/logfmt v0.6.0 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/go-viper/mapstructure/v2 v2.2.1 // indirect
github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 // indirect
github.com/gogo/googleapis v1.4.1 // indirect
Expand All @@ -111,7 +119,9 @@ require (
github.com/google/btree v1.1.3 // indirect
github.com/google/flatbuffers v1.12.1 // indirect
github.com/google/orderedcode v0.0.1 // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/gorilla/websocket v1.5.3 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.1 // indirect
github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c // indirect
github.com/hashicorp/go-hclog v1.5.0 // indirect
github.com/hashicorp/go-immutable-radix v1.3.1 // indirect
Expand Down Expand Up @@ -139,10 +149,9 @@ require (
github.com/petermattis/goid v0.0.0-20240813172612-4fcff4a6cae7 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/prometheus/client_model v0.6.1 // indirect
github.com/prometheus/procfs v0.15.1 // indirect
github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 // indirect
github.com/rogpeppe/go-internal v1.12.0 // indirect
github.com/rogpeppe/go-internal v1.13.1 // indirect
github.com/rs/cors v1.11.1 // indirect
github.com/sagikazarmark/locafero v0.7.0 // indirect
github.com/sasha-s/go-deadlock v0.3.5 // indirect
Expand All @@ -156,6 +165,9 @@ require (
github.com/zondax/ledger-go v0.14.3 // indirect
go.etcd.io/bbolt v1.4.0-alpha.0.0.20240404170359-43604f3112c5 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/auto/sdk v1.1.0 // indirect
go.opentelemetry.io/otel/trace v1.35.0 // indirect
go.opentelemetry.io/proto/otlp v1.5.0 // indirect
go.uber.org/multierr v1.10.0 // indirect
golang.org/x/arch v0.15.0 // indirect
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect
Expand Down
31 changes: 19 additions & 12 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V
github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A=
github.com/go-logfmt/logfmt v0.6.0 h1:wGYYu3uicYdqXVgoYbvnkrPVXkuLM1p1ifugDMEdRi4=
github.com/go-logfmt/logfmt v0.6.0/go.mod h1:WYhtIu8zTZfxdn5+rREduYbwxfcBr/Vr6KEVveWlfTs=
github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
Expand Down Expand Up @@ -370,6 +371,8 @@ github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgf
github.com/grpc-ecosystem/grpc-gateway v1.9.5/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY=
github.com/grpc-ecosystem/grpc-gateway v1.16.0 h1:gmcG1KaJ57LophUzW0Hy8NmPhnMZb4M0+kPpLofRdBo=
github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw=
github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.1 h1:e9Rjr40Z98/clHv5Yg79Is0NtosR5LXRvdr7o/6NwbA=
github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.1/go.mod h1:tIxuGz/9mpox++sgp9fJjHO0+q1X9/UOWd798aAm22M=
github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c h1:6rhixN/i8ZofjG1Y75iExal34USq5p+wiN1tpie8IrU=
github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c/go.mod h1:NMPJylDgVpX0MLRlPy15sqSwOFv/U1GZ2m21JhFfek0=
github.com/hashicorp/consul/api v1.3.0/go.mod h1:MmDNSzIMUjNpY/mQ398R4bk2FnqQLoPndWW5VkKPlCE=
Expand Down Expand Up @@ -638,8 +641,8 @@ github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6So
github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ=
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII=
github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o=
github.com/rs/cors v1.7.0/go.mod h1:gFx+x8UowdsKA9AchylcLynDq+nNFfI8FkUZdN/jGCU=
github.com/rs/cors v1.11.1 h1:eU3gRzXLRK57F5rKMGMZURNdIG4EoAmX8k94r9wXWHA=
github.com/rs/cors v1.11.1/go.mod h1:XyqrcTp5zjWr1wsJ8PIRZssZ8b/WMcMf71DJnit4EMU=
Expand Down Expand Up @@ -739,17 +742,21 @@ go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0=
go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo=
go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA=
go.opentelemetry.io/auto/sdk v1.1.0/go.mod h1:3wSPjt5PWp2RhlCcmmOial7AvC4DQqZb7a7wCow3W8A=
go.opentelemetry.io/otel v1.34.0 h1:zRLXxLCgL1WyKsPVrgbSdMN4c0FMkDAskSTQP+0hdUY=
go.opentelemetry.io/otel v1.34.0/go.mod h1:OWFPOQ+h4G8xpyjgqo4SxJYdDQ/qmRH+wivy7zzx9oI=
go.opentelemetry.io/otel/metric v1.34.0 h1:+eTR3U0MyfWjRDhmFMxe2SsW64QrZ84AOhvqS7Y+PoQ=
go.opentelemetry.io/otel/metric v1.34.0/go.mod h1:CEDrp0fy2D0MvkXE+dPV7cMi8tWZwX3dmaIhwPOaqHE=
go.opentelemetry.io/otel/sdk v1.34.0 h1:95zS4k/2GOy069d321O8jWgYsW3MzVV+KuSPKp7Wr1A=
go.opentelemetry.io/otel/sdk v1.34.0/go.mod h1:0e/pNiaMAqaykJGKbi+tSjWfNNHMTxoC9qANsCzbyxU=
go.opentelemetry.io/otel/sdk/metric v1.34.0 h1:5CeK9ujjbFVL5c1PhLuStg1wxA7vQv7ce1EK0Gyvahk=
go.opentelemetry.io/otel/sdk/metric v1.34.0/go.mod h1:jQ/r8Ze28zRKoNRdkjCZxfs6YvBTG1+YIqyFVFYec5w=
go.opentelemetry.io/otel/trace v1.34.0 h1:+ouXS2V8Rd4hp4580a8q23bg0azF2nI8cqLYnC8mh/k=
go.opentelemetry.io/otel/trace v1.34.0/go.mod h1:Svm7lSjQD7kG7KJ/MUHPVXSDGz2OX4h0M2jHBhmSfRE=
go.opentelemetry.io/otel v1.35.0 h1:xKWKPxrxB6OtMCbmMY021CqC45J+3Onta9MqjhnusiQ=
go.opentelemetry.io/otel v1.35.0/go.mod h1:UEqy8Zp11hpkUrL73gSlELM0DupHoiq72dR+Zqel/+Y=
go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.35.0 h1:QcFwRrZLc82r8wODjvyCbP7Ifp3UANaBSmhDSFjnqSc=
go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.35.0/go.mod h1:CXIWhUomyWBG/oY2/r/kLp6K/cmx9e/7DLpBuuGdLCA=
go.opentelemetry.io/otel/metric v1.35.0 h1:0znxYu2SNyuMSQT4Y9WDWej0VpcsxkuklLa4/siN90M=
go.opentelemetry.io/otel/metric v1.35.0/go.mod h1:nKVFgxBZ2fReX6IlyW28MgZojkoAkJGaE8CpgeAU3oE=
go.opentelemetry.io/otel/sdk v1.35.0 h1:iPctf8iprVySXSKJffSS79eOjl9pvxV9ZqOWT0QejKY=
go.opentelemetry.io/otel/sdk v1.35.0/go.mod h1:+ga1bZliga3DxJ3CQGg3updiaAJoNECOgJREo9KHGQg=
go.opentelemetry.io/otel/sdk/metric v1.35.0 h1:1RriWBmCKgkeHEhM7a2uMjMUfP7MsOF5JpUCaEqEI9o=
go.opentelemetry.io/otel/sdk/metric v1.35.0/go.mod h1:is6XYCUMpcKi+ZsOvfluY5YstFnhW0BidkR+gL+qN+w=
go.opentelemetry.io/otel/trace v1.35.0 h1:dPpEfJu1sDIqruz7BHFG3c7528f6ddfSWfFDVt/xgMs=
go.opentelemetry.io/otel/trace v1.35.0/go.mod h1:WUk7DtFp1Aw2MkvqGdwiXYDZZNvA/1J8o6xRXLrIkyc=
go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI=
go.opentelemetry.io/proto/otlp v1.5.0 h1:xJvq7gMzB31/d406fB8U5CBdyQGw4P399D1aQWU/3i4=
go.opentelemetry.io/proto/otlp v1.5.0/go.mod h1:keN8WnHxOy8PG0rQZjJJ5A2ebUoafqWp0eVQ4yIXvJ4=
go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
go.uber.org/atomic v1.5.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ=
Expand Down
3 changes: 3 additions & 0 deletions server/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,9 @@ func startAPIServer(
}

func startTelemetry(cfg serverconfig.Config) (*telemetry.Metrics, error) {
if cfg.Telemetry.OtlpExporterEnabled {
telemetry.StartOtlpExporter(cfg.Telemetry)
}
return telemetry.New(cfg.Telemetry)
Comment on lines +536 to 539
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Exporter is started without lifecycle management or error propagation

StartOtlpExporter (1) blocks fatal‑logging on failure and (2) launches a goroutine that never stops.
Starting it here means:

  • No way to shut it down during node shutdown (graceDuration, tests, etc.).
  • Potential race: exporter scrapes metrics before telemetry.New registers the Prom sink.

Recommend returning a cancel/cleanup func and wiring it into the existing errgroup, then starting after telemetry.New:

- if cfg.Telemetry.OtlpExporterEnabled {
-     telemetry.StartOtlpExporter(cfg.Telemetry)
- }
- return telemetry.New(cfg.Telemetry)
+ m, err := telemetry.New(cfg.Telemetry)
+ if err != nil {
+     return nil, err
+ }
+ if cfg.Telemetry.OtlpExporterEnabled {
+     cleanup, err := telemetry.StartOtlpExporter(ctx, cfg.Telemetry) // ctx from getCtx
+     if err != nil {
+         return nil, err
+     }
+     g.Go(func() error { <-ctx.Done(); cleanup(); return nil })      // tie to lifecycle
+ }
+ return m, nil

Committable suggestion skipped: line range outside the PR's diff.

}

Expand Down
6 changes: 6 additions & 0 deletions telemetry/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ type Config struct {
// DatadogHostname defines the hostname to use when emitting metrics to
// Datadog. Only utilized if MetricsSink is set to "dogstatsd".
DatadogHostname string `mapstructure:"datadog-hostname"`

// Otlp Exporter fields
OtlpExporterEnabled bool `mapstructure:"otlp-exporter-enabled"`
OtlpCollectorGrpcAddr string `mapstructure:"otlp-collector-grpc-addr"`
PrometheusEndpoint string `mapstructure:"prometheus-endpoint"`
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels like this will need to be automatically populated based on what the configured prometheus metrics endpoint is?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, each chain should probably set their own defaults.

OtlpServiceName string `mapstructure:"otlp-service-name"`
}

// Metrics defines a wrapper around application telemetry functionality. It allows
Expand Down
153 changes: 153 additions & 0 deletions telemetry/otlp_exporter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
package telemetry

import (
"context"
"log"
"math"
"net/http"
"time"

dto "github.com/prometheus/client_model/go"
"github.com/prometheus/common/expfmt"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc"
otmetric "go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/resource"
semconv "go.opentelemetry.io/otel/semconv/v1.21.0"
)

func StartOtlpExporter(cfg Config) {
Copy link
Contributor

Choose a reason for hiding this comment

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

lets have a comment on what this does and how it works

ctx := context.Background()

exporter, err := otlpmetricgrpc.New(ctx,
otlpmetricgrpc.WithEndpoint(cfg.OtlpCollectorGrpcAddr),
otlpmetricgrpc.WithInsecure(),
)
if err != nil {
log.Fatalf("OTLP exporter setup failed: %v", err)
}

Comment on lines +34 to +37
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid log.Fatalf inside library code

log.Fatalf terminates the entire node and makes graceful shutdown impossible. Surface the error instead:

- if err != nil {
-     log.Fatalf("OTLP exporter setup failed: %v", err)
- }
+ if err != nil {
+     return fmt.Errorf("OTLP exporter setup failed: %w", err)
+ }

…and bubble it up to the caller as noted in the server/start.go comment.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if err != nil {
log.Fatalf("OTLP exporter setup failed: %v", err)
}
if err != nil {
return fmt.Errorf("OTLP exporter setup failed: %w", err)
}

res, _ := resource.New(ctx, resource.WithAttributes(
semconv.ServiceName(cfg.OtlpServiceName),
))
Comment on lines +38 to +40
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to check the ignored error here? if not, can we comment why we are able to ignore it?


Comment on lines +38 to +41
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Handle and log the error returned by resource.New

The second return value is currently discarded. If the resource cannot be created, the exporter will run with incomplete metadata.

-res, _ := resource.New(ctx, resource.WithAttributes(
-    semconv.ServiceName(cfg.OtlpServiceName),
-))
+res, rErr := resource.New(ctx, resource.WithAttributes(
+    semconv.ServiceName(cfg.OtlpServiceName),
+))
+if rErr != nil {
+    return fmt.Errorf("failed to initialise OTLP resource: %w", rErr)
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
res, _ := resource.New(ctx, resource.WithAttributes(
semconv.ServiceName(cfg.OtlpServiceName),
))
res, rErr := resource.New(ctx, resource.WithAttributes(
semconv.ServiceName(cfg.OtlpServiceName),
))
if rErr != nil {
return fmt.Errorf("failed to initialise OTLP resource: %w", rErr)
}

meterProvider := metric.NewMeterProvider(
metric.WithReader(metric.NewPeriodicReader(exporter,
metric.WithInterval(15*time.Second))),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the interval be configurable here?

metric.WithResource(res),
)
otel.SetMeterProvider(meterProvider)
meter := otel.Meter("cosmos-sdk-otlp-exporter")
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: move to const


gauges := make(map[string]otmetric.Float64Gauge)
histograms := make(map[string]otmetric.Float64Histogram)

go func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

how does this behave when shutting a node down? Do we want any kind of graceful shutdown here?

for {
if err := scrapeAndPushMetrics(ctx, cfg.PrometheusEndpoint, meter, gauges, histograms); err != nil {
log.Printf("error scraping metrics: %v", err)
}
time.Sleep(15 * time.Second)
}
}()
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

My understanding is the periodicReader does the pushing to the collector, but this function does the reading prometheus metrics and converting them into otlp metrics.

I renamed the function to scrapePrometheusMetrics for clarity

}

func scrapeAndPushMetrics(ctx context.Context, promEndpoint string, meter otmetric.Meter, gauges map[string]otmetric.Float64Gauge, histograms map[string]otmetric.Float64Histogram) error {
resp, err := http.Get(promEndpoint)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there not a better way to hook up otlp and prometheus than making a get request to the local endpoint?

Copy link
Author

Choose a reason for hiding this comment

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

I changed this to use the prometheus DefaultGatherer, but that will only gather the metrics registered with the DefaultRegisterer. But it seems like all comet metrics are there and the cosmos SDK does not actually expose any way to use another registerer, so it should be good.

if err != nil {
return err
}
defer resp.Body.Close()

parser := expfmt.TextParser{}
metricsFamilies, err := parser.TextToMetricFamilies(resp.Body)
if err != nil {
return err
}

for name, mf := range metricsFamilies {
for _, m := range mf.GetMetric() {

Check failure on line 71 in telemetry/otlp_exporter.go

View workflow job for this annotation

GitHub Actions / golangci-lint

File is not properly formatted (gofumpt)

Check failure on line 71 in telemetry/otlp_exporter.go

View workflow job for this annotation

GitHub Actions / Analyze

File is not properly formatted (gofumpt)
switch {
case m.Gauge != nil:
recordGauge(ctx, meter, gauges, name, mf.GetHelp(), m.Gauge.GetValue())

case m.Counter != nil:
recordGauge(ctx, meter, gauges, name, mf.GetHelp(), m.Counter.GetValue())

case m.Histogram != nil:
recordHistogram(ctx, meter, histograms, name, mf.GetHelp(), m.Histogram)

case m.Summary != nil:
continue // TODO: decide whether to support

default:
continue
}
}
}

return nil
}

func recordGauge(ctx context.Context, meter otmetric.Meter, gauges map[string]otmetric.Float64Gauge, name, help string, val float64) {
g, ok := gauges[name]
if !ok {
var err error
g, err = meter.Float64Gauge(name, otmetric.WithDescription(help))
if err != nil {
log.Printf("failed to create gauge %q: %v", name, err)
return
}
gauges[name] = g
}
g.Record(ctx, val)
}

func recordHistogram(ctx context.Context, meter otmetric.Meter, histograms map[string]otmetric.Float64Histogram, name, help string, h *dto.Histogram) {
boundaries := make([]float64, 0, len(h.Bucket)-1) // excluding +Inf
bucketCounts := make([]uint64, 0, len(h.Bucket))

for _, bucket := range h.Bucket {
if math.IsInf(bucket.GetUpperBound(), +1) {
continue // Skip +Inf bucket boundary explicitly
}
Comment on lines +114 to +116
Copy link
Contributor

@technicallyty technicallyty Mar 31, 2025

Choose a reason for hiding this comment

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

why do we skip this? i know it says +Inf boundary, but as someone who is unfamiliar with OTL, i am not sure what the significance is

boundaries = append(boundaries, bucket.GetUpperBound())
bucketCounts = append(bucketCounts, bucket.GetCumulativeCount())
}

hist, ok := histograms[name]
if !ok {
var err error
hist, err = meter.Float64Histogram(
name,
otmetric.WithDescription(help),
otmetric.WithExplicitBucketBoundaries(boundaries...),
)
if err != nil {
log.Printf("failed to create histogram %s: %v", name, err)
return
}
histograms[name] = hist
}

prevCount := uint64(0)
for i, count := range bucketCounts {
countInBucket := count - prevCount
prevCount = count

// Explicitly record the mid-point of the bucket as approximation:
var value float64
if i == 0 {
value = boundaries[0] / 2.0

Check notice

Code scanning / CodeQL

Floating point arithmetic Note

Floating point arithmetic operations are not associative and a possible source of non-determinism
} else {
value = (boundaries[i-1] + boundaries[i]) / 2.0

Check notice

Code scanning / CodeQL

Floating point arithmetic Note

Floating point arithmetic operations are not associative and a possible source of non-determinism

Check notice

Code scanning / CodeQL

Floating point arithmetic Note

Floating point arithmetic operations are not associative and a possible source of non-determinism
}

// Record `countInBucket` number of observations explicitly (approximation):
for j := uint64(0); j < countInBucket; j++ {
hist.Record(ctx, value)
}
Comment on lines +150 to +152
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for j := uint64(0); j < countInBucket; j++ {
hist.Record(ctx, value)
}
for range countInBucket {
hist.Record(ctx, value)
}

}
}
Loading