Skip to content

feat: otel go metrics setup#200

Merged
tim-mwangi merged 8 commits intomainfrom
metrics-round-1
Jan 30, 2023
Merged

feat: otel go metrics setup#200
tim-mwangi merged 8 commits intomainfrom
metrics-round-1

Conversation

@tim-mwangi
Copy link
Copy Markdown
Collaborator

@tim-mwangi tim-mwangi commented Jan 24, 2023

Description

Set up otel go metrics reporting and add a RequestCount metric for the http handler instrumentation. This should enable metrics to be created and exported just like spans are. By default we will export to the otlp endpoint spans are exported to but this can be overridden using a metrics endpoint config.

Testing

Tested locally.

Checklist:

  • [ ✅ ] My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • [✅ ] Any dependent changes have been merged and published in downstream modules

Set up otel go metrics reporting and add a RequestCount metric for the http handler instrumentation.
@tim-mwangi tim-mwangi requested review from prodion23 and ryanericson and removed request for prodion23 and ryanericson January 24, 2023 18:53
@tim-mwangi tim-mwangi marked this pull request as draft January 24, 2023 18:55
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 25, 2023

Codecov Report

Merging #200 (1cebe35) into main (c886e58) will increase coverage by 0.53%.
The diff coverage is 76.69%.

@@            Coverage Diff             @@
##             main     #200      +/-   ##
==========================================
+ Coverage   64.99%   65.52%   +0.53%     
==========================================
  Files          49       49              
  Lines        1774     1868      +94     
==========================================
+ Hits         1153     1224      +71     
- Misses        570      587      +17     
- Partials       51       57       +6     
Impacted Files Coverage Δ
...nstrumentation/hypertrace/net/hyperhttp/handler.go 0.00% <0.00%> (ø)
instrumentation/opentelemetry/init.go 80.06% <72.60%> (-2.45%) ⬇️
sdk/instrumentation/net/http/handler.go 28.69% <84.21%> (+2.84%) ⬆️
...nstrumentation/opencensus/net/hyperhttp/handler.go 100.00% <100.00%> (ø)
...opentelemetry/github.com/gin-gonic/hypergin/gin.go 89.36% <100.00%> (+0.23%) ⬆️
...n/opentelemetry/github.com/gorilla/hypermux/mux.go 65.21% <100.00%> (+1.58%) ⬆️
...rumentation/opentelemetry/net/hyperhttp/handler.go 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@tim-mwangi tim-mwangi marked this pull request as ready for review January 25, 2023 20:14
"github.com/hypertrace/goagent/sdk/filter"
internalconfig "github.com/hypertrace/goagent/sdk/internal/config"
"github.com/hypertrace/goagent/sdk/internal/container"
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I need to move this otel specific logic out of the sdk since this is meant to be a hypertrace sdk that can be adapted by any telemetry framework. But can do that separately.

metricsExporterFactory := makeMetricsExporterFactory(cfg)
metricsExporter, err := metricsExporterFactory()
if err != nil {
log.Fatal(err)
Copy link
Copy Markdown
Collaborator

@ryanericson ryanericson Jan 27, 2023

Choose a reason for hiding this comment

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

This means if metrics backend is down instrumented app won't be able to start? Better to fail silently?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

If the metrics/traces collector is down and you bring up the go app with this agent in it, it comes up without issue. You do get context deadline messages since you cannot export anything. After you bring up the collector the spans and metrics start being sent.

@tim-mwangi tim-mwangi merged commit 6bbe09c into main Jan 30, 2023
@tim-mwangi tim-mwangi deleted the metrics-round-1 branch January 30, 2023 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants