From 24685b499f29d16b62fa105a9a42f34fb18de4ae Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Mon, 28 Sep 2026 10:20:09 -0700 Subject: [PATCH] echo-otel/v5: rename module, error.type per semconv, Echo.Pre route, v5.0.0 Align with echo-jwt: MAJOR version tracks the Echo version. This is the Echo v5 line, github.com/labstack/echo-otel/v5; Echo v4 is supported on the v4 branch as github.com/labstack/echo-otel/v4. - module github.com/labstack/echo-otel/v5, ScopeName = module path, Version 5.0.0 - minimal Echo v5.2.1 (unchanged), CI also tests the latest Echo v5 - error.type on spans and metrics per the HTTP semantic conventions (#10); semconv v1.40.0, which unwraps fmt.Errorf wrappers for error.type - set http.route and span name when the middleware is added with Echo.Pre; http.route is always the Echo route, never an outer http.ServeMux pattern - metrics only carry low cardinality attributes: server.address, server.port and http.request.method_original stay on spans (Opt-In for metrics) - with ServerName set, server.port no longer comes from the Host header - do not record an unknown (-1) request body size - record a panicking handler (error.type "panic", also after a 4xx response) and re-panic; a panic in a middleware callback keeps the original value; runtime.Goexit and http.ErrAbortHandler are handled; a Recover middleware added after this middleware (PanicStackError) is reported as "panic" - a panic value that carries a status (echo.ErrUnauthorized) records that status - no http.response.status_code when no response status was sent - network.protocol.version "2"/"3" for HTTP/2 and HTTP/3 - http.request.body.size / http.response.body.size are Opt-In on spans and no longer set there; the body size metrics stay - reject a ServerName without a host (for example ":8080") - fix the example module path; build and vet the example in CI - rename extrator.go to extractor.go; CI on Go 1.25, 1.26 and 1.27 - add a middleware benchmark; tests run on Windows (temp dir via the form) - OpenTelemetry v1.46.0 - README: versioning, migrating from echo-opentelemetry and otelecho, custom error handler, error.type, spans, known limitations, Renovate major updates --- .github/workflows/checks.yml | 8 +- .github/workflows/echo.yml | 17 +- .gitignore | 3 + Makefile | 6 +- README.md | 221 +++++++++++- example/go.mod | 20 +- example/go.sum | 56 ++- example/main.go | 2 +- extrator.go => extractor.go | 178 +++++++--- extrator_test.go => extractor_test.go | 180 +++++++++- go.mod | 24 +- go.sum | 55 ++- otel.go | 134 ++++++-- otel_behavior_test.go | 469 ++++++++++++++++++++++++++ otel_test.go | 131 ++++++- version.go | 2 +- 16 files changed, 1310 insertions(+), 196 deletions(-) rename extrator.go => extractor.go (76%) rename extrator_test.go => extractor_test.go (81%) create mode 100644 otel_behavior_test.go diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index fb0c204..6a56ec1 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -14,7 +14,7 @@ permissions: env: # run static analysis only with the latest Go version - LATEST_GO_VERSION: "1.26" + LATEST_GO_VERSION: "1.27" jobs: check: @@ -23,12 +23,16 @@ jobs: - name: Checkout Code uses: actions/checkout@v5 - - name: Set up Go ${{ matrix.go }} + - name: Set up Go ${{ env.LATEST_GO_VERSION }} uses: actions/setup-go@v6 with: go-version: ${{ env.LATEST_GO_VERSION }} check-latest: true + - name: Build example + working-directory: example + run: go build ./... && go vet ./... + - name: Run golint run: | go install golang.org/x/lint/golint@latest diff --git a/.github/workflows/echo.yml b/.github/workflows/echo.yml index b5bda26..65f8acc 100644 --- a/.github/workflows/echo.yml +++ b/.github/workflows/echo.yml @@ -14,7 +14,7 @@ permissions: env: # run coverage and benchmarks only with the latest Go version - LATEST_GO_VERSION: "1.26" + LATEST_GO_VERSION: "1.27" jobs: test: @@ -25,7 +25,7 @@ jobs: # Echo tests with last four major releases (unless there are pressing vulnerabilities) # As we depend on `golang.org/x/` libraries which only support last 2 Go releases we could have situations when # we derive from last four major releases promise. - go: [ "1.25", "1.26" ] + go: [ "1.25", "1.26", "1.27" ] name: ${{ matrix.os }} @ Go ${{ matrix.go }} runs-on: ${{ matrix.os }} steps: @@ -40,6 +40,14 @@ jobs: - name: Run Tests run: go test -race --coverprofile=coverage.coverprofile --covermode=atomic ./... + - name: Run Tests with the latest Echo v5 + if: matrix.os == 'ubuntu-latest' + env: + GOTOOLCHAIN: local + run: | + go get github.com/labstack/echo/v5@latest + go test -race ./... + - name: Upload coverage to Codecov if: success() && matrix.go == env.LATEST_GO_VERSION && matrix.os == 'ubuntu-latest' uses: codecov/codecov-action@v5 @@ -49,11 +57,12 @@ jobs: benchmark: needs: test + if: github.event_name == 'pull_request' name: Benchmark comparison runs-on: ubuntu-latest steps: - name: Checkout Code (Previous) - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: ${{ github.base_ref }} path: previous @@ -63,7 +72,7 @@ jobs: with: path: new - - name: Set up Go ${{ matrix.go }} + - name: Set up Go ${{ env.LATEST_GO_VERSION }} uses: actions/setup-go@v6 with: go-version: ${{ env.LATEST_GO_VERSION }} diff --git a/.gitignore b/.gitignore index dbadf3b..eadcfa7 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,6 @@ vendor *.iml *.out .vscode + +# example binary +/example/example diff --git a/Makefile b/Makefile index b2e9bfe..1746d93 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -PKG := "github.com/labstack/echo-opentelemetry" +PKG := "github.com/labstack/echo-otel/v5" PKG_LIST := $(shell go list ${PKG}/...) .DEFAULT_GOAL := check @@ -35,6 +35,6 @@ format: ## Format the source code help: ## Display this help screen @grep -h -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' -goversion ?= "1.26" -test_version: ## Run tests inside Docker with given version (defaults to 1.26 oldest supported). Example: make test_version goversion=1.26 +goversion ?= "1.25" +test_version: ## Run tests inside Docker with given version (defaults to 1.25 oldest supported). Example: make test_version goversion=1.25 @docker run --rm -it -v $(shell pwd):/project golang:$(goversion) /bin/sh -c "cd /project && make race" diff --git a/README.md b/README.md index 86fa5e7..46eebc3 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ -[![Sourcegraph](https://sourcegraph.com/github.com/labstack/echo-opentelemetry/-/badge.svg?style=flat-square)](https://sourcegraph.com/github.com/labstack/echo-opentelemetry?badge) -[![GoDoc](http://img.shields.io/badge/go-documentation-blue.svg?style=flat-square)](https://pkg.go.dev/github.com/labstack/echo-opentelemetry) -[![Go Report Card](https://goreportcard.com/badge/github.com/labstack/echo-opentelemetry?style=flat-square)](https://goreportcard.com/report/github.com/labstack/echo-opentelemetry) -[![License](http://img.shields.io/badge/license-mit-blue.svg?style=flat-square)](https://raw.githubusercontent.com/labstack/echo-opentelemetry/main/LICENSE) +[![Sourcegraph](https://sourcegraph.com/github.com/labstack/echo-otel/-/badge.svg?style=flat-square)](https://sourcegraph.com/github.com/labstack/echo-otel?badge) +[![GoDoc](http://img.shields.io/badge/go-documentation-blue.svg?style=flat-square)](https://pkg.go.dev/github.com/labstack/echo-otel/v5) +[![Go Report Card](https://goreportcard.com/badge/github.com/labstack/echo-otel?style=flat-square)](https://goreportcard.com/report/github.com/labstack/echo-otel) +[![License](http://img.shields.io/badge/license-mit-blue.svg?style=flat-square)](https://raw.githubusercontent.com/labstack/echo-otel/main/LICENSE) # Echo OpenTelemetry (OTel) middleware @@ -10,24 +10,34 @@ * [OpenTelemetry HTTP spec](https://opentelemetry.io/docs/specs/semconv/http/) * [HTTP metrics spec](https://opentelemetry.io/docs/specs/semconv/http/http-metrics/) - ## Versioning -* version `v0.x.y` tracks the latest Echo version (`v5`). -* `main` branch is compatible with the latest Echo version (`v5`). +This repository does not use semantic versioning. MAJOR version tracks which Echo version should be used. MINOR version +tracks API changes (possibly backwards incompatible) and PATCH version is incremented for fixes. + +| Echo | Module | Branch | Minimal Echo version | +|---|---|---|---| +| v5 | `github.com/labstack/echo-otel/v5` | `main` | `v5.2.1` | +| v4 | `github.com/labstack/echo-otel/v4` | `v4` | `v4.15.4` | + +This is the `main` branch, for Echo v5. `github.com/labstack/echo-opentelemetry` (`v0.0.x`, Echo v5) is the +previous name of this project and is deprecated. + +Always include the MAJOR version suffix (`/v5` or `/v4`) in `go get` and imports. Without it, +`go get github.com/labstack/echo-otel` fails. ## Usage Add OpenTelemetry middleware dependency with go modules ```bash -go get github.com/labstack/echo-opentelemetry +go get github.com/labstack/echo-otel/v5 ``` Use as an import statement ```go -import echootel "github.com/labstack/echo-opentelemetry" +import echootel "github.com/labstack/echo-otel/v5" ``` Add middleware in simplified form, by providing only the server name @@ -46,9 +56,200 @@ e.Use(echootel.NewMiddlewareWithConfig(echootel.Config{ Retrieving the tracer from the Echo context ```go -tp, err := echo.ContextGet[trace.Tracer](c, echootel.TracerKey) +tracer, err := echo.ContextGet[trace.Tracer](c, echootel.TracerKey) ``` ## Full example See [example](example/main.go) + +## Custom error handler + +The middleware resolves the response status code for returned errors with `echo.ResolveResponseStatus`: the status of +an already sent response, the `StatusCode()` of the returned error (for example `echo.HTTPError`), or 500. If you +use a custom `HTTPErrorHandler` that maps errors to other status codes, let the middleware run the error handler, so it +reports the status code that was actually sent: + +```go +e.Use(echootel.NewMiddlewareWithConfig(echootel.Config{ + ServerName: "app.example.com", + OnNextError: func(c *echo.Context, err error) { c.Echo().HTTPErrorHandler(c, err) }, +})) +``` + +The error is still returned, so the error handler is called again; make it return early when the response is already +committed (`resp, err := echo.UnwrapResponse(c.Response()); err == nil && resp.Committed`). Echo's +`ProblemDetailsHTTPErrorHandler` (Echo v5.4.0+) does this; use the snippet above with it too, as it resolves the status +of `ProblemErrorer` errors and of `*echo.ProblemError` wrapped in another error differently. + +## Migrating from otelecho + +`go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho` is deprecated and removed from +opentelemetry-go-contrib in favor of this library. +For Echo v5 use `github.com/labstack/echo-otel/v5`. For Echo v4 use `github.com/labstack/echo-otel/v4`, there is no +need to migrate to Echo v5 first. + +Replace `otelecho.Middleware("my-server", opts...)` with `echootel.NewMiddleware("my-server")`, or with +`echootel.NewMiddlewareWithConfig(echootel.Config{...})` when you use options. +The server name is used as `server.address` and `server.port`, so it must be a host name with an optional port, +for example `api.example.com` or `api.example.com:8080` (otelecho accepted any string); `NewMiddleware` panics on an +invalid value such as `:8080` (no host), `Config.ToMiddleware` returns an error, and an empty `ServerName` uses the +request `Host`: + +| otelecho option | echootel.Config field | +|---|---| +| `otelecho.WithTracerProvider(tp)` | `TracerProvider: tp` | +| `otelecho.WithMeterProvider(mp)` | `MeterProvider: mp` | +| `otelecho.WithPropagators(p)` | `Propagators: p` | +| `otelecho.WithSkipper(s)` | `Skipper: s` | +| `otelecho.WithMetricAttributeFn(f)` | `MetricAttributes` (see below) | +| `otelecho.WithEchoMetricAttributeFn(f)` | `MetricAttributes` (see below) | +| `otelecho.WithOnError(f)` | `OnNextError` (and `OnExtractionError` for request extraction failures) | + +Also note: + +* `MetricAttributes` replaces the default metric attributes (otelecho appended to them), so return + `append(v.MetricAttributes(), extra...)`. +* otelecho called the Echo error handler from the middleware by default (`c.Error(err)`); echootel does not. See + [Custom error handler](#custom-error-handler) for how to get the same behavior. +* The instrumentation scope name changes from + `go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho` + to `github.com/labstack/echo-otel/v5`. Update dashboards and alerts that filter on it. +* The `echo.error` span attribute is not set; use `error.type` and the span status instead. + +## Migrating from echo-opentelemetry + +`github.com/labstack/echo-opentelemetry` (`v0.0.x`) is the previous name of this project. It is deprecated and receives +no more changes. Change the import path; the API is the same, except that a `ServerName` without a host is rejected +(see below): + +```go +import echootel "github.com/labstack/echo-otel/v5" +``` + +Telemetry changes compared to `echo-opentelemetry` `v0.0.3`: + +* The instrumentation scope name changes from `github.com/labstack/echo-opentelemetry` to + `github.com/labstack/echo-otel/v5`. Update dashboards and alerts that filter on it. +* `error.type` is no longer set for 4xx responses (it used to be `*echo.HTTPError` for a returned HTTPError, and + `*echo.httpError` for Echo's own errors such as `echo.ErrNotFound`). +* A returned 5xx HTTPError reports the status code (`500`) instead of `*echo.HTTPError`. +* A 5xx response written without an error (for example `c.String(500, ...)`) now reports `error.type`. +* A returned error with a `StatusCode() int` method reports the status code for a 5xx response instead of its Go type. +* `error.type` is also added to metrics, see [Errors](#errors). +* `http.route` and the span name are set when the middleware is added with `Echo.Pre`. +* An error wrapped with `fmt.Errorf("...: %w", err)` reports the wrapped error's type (for example `*net.OpError`) + instead of `*fmt.wrapError`, and an `ErrorType() string` method in the error chain is used when present. +* `error.type` is now one of the span end attributes: a `Config.SpanEndAttributes` callback must append to the `attr` + argument, otherwise `error.type` is dropped (v0.0.3 set it separately). +* A status code of 600-999 now reports `error.type` (the code). +* Metrics no longer have `server.address`, `server.port` and `http.request.method_original`; their values come from + the request and would make metric cardinality unbounded. They stay on spans. See [Metrics](#metrics). +* With `ServerName` set, `server.port` comes from `ServerName` only, not from the `Host` header. +* An unknown request body size (for example a chunked request) is not recorded, instead of `-1`. +* `http.route` is always the Echo route, never the pattern of an outer `http.ServeMux`. +* A panic in a handler is recorded (`error.type` `panic`) and then re-panicked, so telemetry is recorded also when a + Recover middleware is added before this middleware. With a Recover middleware added after this middleware, the + returned `*middleware.PanicStackError` is reported as `panic` instead of its Go type. See [Errors](#errors). +* A `ServerName` without a host (for example `:8080`) is rejected: `NewMiddleware` panics and `Config.ToMiddleware` + returns an error. v0.0.3 accepted it. +* `network.protocol.version` is `2` for HTTP/2 and `3` for HTTP/3, instead of `2.0` and `3.0`. +* Spans no longer have `http.request.body.size` and `http.response.body.size` (Opt-In for spans in the semantic + conventions). The body size metrics stay. See [Spans](#spans) for how to add them back. + +## Errors + +A request that ends with an error sets `error.type` on the span and on the metrics, as the semantic conventions +require. The rules match the span status: + +* a 4xx response is not an error for server spans: no `error.type`, +* a 5xx response without a returned error, or with a returned error that carries the status code (`echo.HTTPError` + or any error with a `StatusCode() int` method, also when wrapped), reports the status code, for example `500`, +* any other returned error reports its Go type, for example `*net.OpError`. Errors created with + `fmt.Errorf("...: %w", err)` are unwrapped first; errors joining several errors (`errors.Join`, `fmt.Errorf` with + more than one `%w`) report their own type. An `ErrorType() string` method anywhere in the error chain takes + precedence; keep its values low cardinality, as `error.type` is also a metric attribute. +* an invalid status code without a returned error reports `_OTHER` (below 100 or above 999) or the code itself + (600-999), +* a panic reports `panic`, also when a 4xx response was already sent. + +The middleware records a panic and then re-panics, so add a Recover middleware before this middleware +(`e.Use(middleware.Recover())` first). The recorded status is the status of the response that was already sent, the +status of a panic value that carries one (for example `echo.ErrUnauthorized`), or 500. A Recover middleware added after +this middleware (with the default configuration) returns the panic as a `*middleware.PanicStackError`, which is also +reported as `panic`. + +`error.type` is part of the span end attributes. A `Config.SpanEndAttributes` callback must append to the `attr` +argument and return it, otherwise `error.type` and the other end attributes are dropped. + +## Spans + +Spans follow the [HTTP server span](https://opentelemetry.io/docs/specs/semconv/http/http-spans/#http-server) semantic +conventions, with these exceptions: + +* `url.query` is not set. The query string can contain sensitive data, and the semantic conventions require redacting + it. `http.request.body.size` and `http.response.body.size` are Opt-In and not set either. +* `http.response.status_code` is not set when no response was sent, for example after an `http.ErrAbortHandler` panic. + +`client.address` comes from `c.RealIP()`: the connection's remote address, or the result of `Echo.IPExtractor` when +it is set. + +Add attributes with `SpanStartAttributes` and `SpanEndAttributes`. Both callbacks must append to the `attr` argument and +return it: + +```go +SpanStartAttributes: func(c *echo.Context, v *echootel.Values, attr []attribute.KeyValue) []attribute.KeyValue { + if q := c.Request().URL.RawQuery; q != "" { + attr = append(attr, semconv.URLQuery(redactQuery(q))) // redactQuery is your own function + } + return attr +}, +SpanEndAttributes: func(c *echo.Context, v *echootel.Values, attr []attribute.KeyValue) []attribute.KeyValue { + if v.HTTPRequestBodySize >= 0 { + attr = append(attr, semconv.HTTPRequestBodySize(int(v.HTTPRequestBodySize))) + } + return append(attr, semconv.HTTPResponseBodySize(int(v.HTTPResponseBodySize))) +}, +``` + +## Metrics + +The middleware records `http.server.request.duration`, `http.server.request.body.size` and +`http.server.response.body.size` with these attributes: `http.request.method`, `url.scheme`, `http.route`, +`network.protocol.name`, `network.protocol.version`, `http.response.status_code` and `error.type`. + +`server.address`, `server.port` and `http.request.method_original` are Opt-In for metrics in the semantic conventions +and are not added: the `Host` header and the request method are chosen by the client and would make metric cardinality +unbounded. Add attributes with a known set of values with `MetricAttributes`, for example: + +```go +MetricAttributes: func(c *echo.Context, v *echootel.Values) []attribute.KeyValue { + return append(v.MetricAttributes(), semconv.ServerAddress("api.example.com")) +}, +``` + +## Known limitations + +* Hijacked connections (for example WebSocket upgrades) are reported with the status code that Echo's response has, + usually 200, not 101. +* A panic without a Recover middleware is recorded with status 500, but `net/http` closes the connection without + sending a response. + +## Dependency update bots + +Renovate proposes a new MAJOR version (for example `github.com/labstack/echo-otel/v4` to `/v5`) as an update. A new +MAJOR version of this library needs the same MAJOR version of Echo, so update Echo first. To stay on the current Echo +version, disable major updates for this library: + +```json +{ + "packageRules": [ + { + "matchManagers": ["gomod"], + "matchPackageNames": ["github.com/labstack/echo-otel/v4", "github.com/labstack/echo-otel/v5"], + "matchUpdateTypes": ["major"], + "enabled": false + } + ] +} +``` diff --git a/example/go.mod b/example/go.mod index 22263af..086ff5a 100644 --- a/example/go.mod +++ b/example/go.mod @@ -1,25 +1,25 @@ -module github.com/labstack/echo-opentelemetry/echootel/example +module github.com/labstack/echo-otel/v5/example go 1.25.6 -replace github.com/labstack/echo-opentelemetry/echootel => ../ +replace github.com/labstack/echo-otel/v5 => ../ require ( - github.com/labstack/echo-opentelemetry/echootel v0.0.0-00010101000000-000000000000 - github.com/labstack/echo/v5 v5.2.1 - go.opentelemetry.io/otel v1.44.0 - go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.44.0 - go.opentelemetry.io/otel/sdk v1.44.0 - go.opentelemetry.io/otel/trace v1.44.0 + github.com/labstack/echo-otel/v5 v5.0.0 + github.com/labstack/echo/v5 v5.4.0 + go.opentelemetry.io/otel v1.46.0 + go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.46.0 + go.opentelemetry.io/otel/sdk v1.46.0 + go.opentelemetry.io/otel/trace v1.46.0 ) require ( github.com/cespare/xxhash/v2 v2.3.0 // indirect - github.com/go-logr/logr v1.4.3 // indirect + github.com/go-logr/logr v1.4.4 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/google/uuid v1.6.0 // indirect go.opentelemetry.io/auto/sdk v1.2.1 // indirect - go.opentelemetry.io/otel/metric v1.44.0 // indirect + go.opentelemetry.io/otel/metric v1.46.0 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/time v0.15.0 // indirect ) diff --git a/example/go.sum b/example/go.sum index f4e8071..600fdf2 100644 --- a/example/go.sum +++ b/example/go.sum @@ -1,47 +1,43 @@ github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= -github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= -github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= -github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-logr/logr v1.4.4 h1:tG4xh9yMsRCAiodLVTxyrkzSZ9+o0L1Kg/+cPVcbP/8= +github.com/go-logr/logr v1.4.4/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/labstack/echo/v5 v5.2.1 h1:TzpIksY6zLMzV0T0ycYbvTEoj9w6o6AcL5twg182VTY= -github.com/labstack/echo/v5 v5.2.1/go.mod h1:SyvlSdObGjRXeQfCCXW/sybkZdOOQZBmpKF0bvALaeo= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= -github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= -github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +github.com/labstack/echo/v5 v5.4.0 h1:iY674460IvSmUcj7MziL3YrgyTClghz5BpdZoKiGxR4= +github.com/labstack/echo/v5 v5.4.0/go.mod h1:4iEGNQiPPZnkfYpNR/L6fINd3NLiGWUD5+eBotFALas= +github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE= +github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg= go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= -go.opentelemetry.io/contrib/propagators/b3 v1.42.0 h1:B2Pew5ufEtgkjLF+tSkXjgYZXQr9m7aCm1wLKB0URbU= -go.opentelemetry.io/contrib/propagators/b3 v1.42.0/go.mod h1:iPgUcSEF5DORW6+yNbdw/YevUy+QqJ508ncjhrRSCjc= -go.opentelemetry.io/otel v1.44.0 h1:JjwHmHpA4iZ3wBxluu2fbbE7j4kqlE8jXyAyPXH7HqU= -go.opentelemetry.io/otel v1.44.0/go.mod h1:BMgjTHL9WPRlRjL2oZCBTL4whCGtXch2H4BhOPIAyYc= -go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.44.0 h1:bl2S7Ubua0Nms+D/gAmznQTd4dxxMA93aKbcpKqiTCs= -go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.44.0/go.mod h1:L0hRV50XdVIODHUfWEqGRCXQvj2rV82STVo12FMFBU0= -go.opentelemetry.io/otel/metric v1.44.0 h1:1w0gILTcHdr3YI+ixLyjemwrVnsMURbTZFrSYCdDdmc= -go.opentelemetry.io/otel/metric v1.44.0/go.mod h1:8O7hanEPBNgEMmybD3s2VBKcgWOCsA6tzHBPODAiquo= -go.opentelemetry.io/otel/sdk v1.44.0 h1:nHYwb9lK+fJPU/dnT6s7W7Z8itMWyqrnVfbheVYrZ58= -go.opentelemetry.io/otel/sdk v1.44.0/go.mod h1:Osuydd3Se74nqjAKxid74N5eC+jfEqfTegHRnq58oK0= -go.opentelemetry.io/otel/sdk/metric v1.44.0 h1:3LlKgI+VjbVsjNRFZJZAJ30WjXC5VkNRks6si09iEfI= -go.opentelemetry.io/otel/sdk/metric v1.44.0/go.mod h1:5B5pMARnXxKhltooO4xUuCBorl65a4EpnTalObqOigA= -go.opentelemetry.io/otel/trace v1.44.0 h1:jxF5CsGYCe74MCRx2X4g7WsY/VBKRqqpNvXlX/6gtIk= -go.opentelemetry.io/otel/trace v1.44.0/go.mod h1:oLl1jrMQAVo6v3GAggN+1VH9VIz9iUSvW53sW1Q8PIE= +go.opentelemetry.io/contrib/propagators/b3 v1.46.0 h1:OFVqWObn7xLIbOjE/koO0LS9fZJNgAyBD0msA+UQAoc= +go.opentelemetry.io/contrib/propagators/b3 v1.46.0/go.mod h1:t/d64xy7xuuEDJN/4ThqohLgRhIuQxL9y7P1v02bYuM= +go.opentelemetry.io/otel v1.46.0 h1:FHt5/CDyVxi/8IM1CH7VE/rRgq3kLHa2mSTVMO8AWyc= +go.opentelemetry.io/otel v1.46.0/go.mod h1:Gj3SEScelsNC45tp4nSxRYlS+f5iez7W8XPMCt905kE= +go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.46.0 h1:KdRxPiAoMptR3vfWzvjjvutTsSiwbC2uG0496rzZNfo= +go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.46.0/go.mod h1:K/qSA+3G7Eovxi4K09wzrAgkWRnosS0DAOZeEpve7sM= +go.opentelemetry.io/otel/metric v1.46.0 h1:yBnkXvgV7AXFILZc5K6IZe/CBFF3OS7BJ8ov6/lj0K8= +go.opentelemetry.io/otel/metric v1.46.0/go.mod h1:iPmdWqifKUdzziPkvvzIJXITl56fQx2mGM/DHLB3/2o= +go.opentelemetry.io/otel/sdk v1.46.0 h1:h5CNQQjEbuQXY/JfZtgt3i7HVFV3aHPO2OAwO2eTYPI= +go.opentelemetry.io/otel/sdk v1.46.0/go.mod h1:GAERFXFt5SYCEB+YiKUbMBeza6UaDH7GmGOZEfh2gSM= +go.opentelemetry.io/otel/sdk/metric v1.46.0 h1:0piZ26EG4RBfebb2jhDH6ERCYHoVWduc3kLgPCwSnSE= +go.opentelemetry.io/otel/sdk/metric v1.46.0/go.mod h1:I1PbKrdVc8Qu8HYVDNtqVIwLwjNrhsV/uFuxfwg8mO4= +go.opentelemetry.io/otel/trace v1.46.0 h1:OULy7ccdJnZtJ0UDYFOIGaCmiWzJ8Vi2G/Rsu60qs1c= +go.opentelemetry.io/otel/trace v1.46.0/go.mod h1:J7GAXweO77XSFkB/rmAqk9D6ihszhFjLU+d9WuUxDLI= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= -golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= -golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= +go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= +go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg= +golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE= +golang.org/x/net v0.57.0/go.mod h1:KpXc8iv+r3XplLAG/f7Jsf9RPszJzdR0f58q9vGOuEU= golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/text v0.33.0 h1:B3njUFyqtHDUI5jMn1YIr5B0IE2U0qck04r6d4KPAxE= -golang.org/x/text v0.33.0/go.mod h1:LuMebE6+rBincTi9+xWTY8TztLzKHc/9C1uBCG27+q8= +golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= +golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= -gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= -gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/example/main.go b/example/main.go index 2024a72..43e6b4f 100644 --- a/example/main.go +++ b/example/main.go @@ -11,7 +11,7 @@ import ( "log/slog" "net/http" - "github.com/labstack/echo-opentelemetry/echootel" + echootel "github.com/labstack/echo-otel/v5" "github.com/labstack/echo/v5" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" diff --git a/extrator.go b/extractor.go similarity index 76% rename from extrator.go rename to extractor.go index bd10bad..c2f952f 100644 --- a/extrator.go +++ b/extractor.go @@ -14,8 +14,8 @@ import ( "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/metric" - semconv "go.opentelemetry.io/otel/semconv/v1.39.0" - "go.opentelemetry.io/otel/semconv/v1.39.0/httpconv" + semconv "go.opentelemetry.io/otel/semconv/v1.40.0" + "go.opentelemetry.io/otel/semconv/v1.40.0/httpconv" ) /* @@ -33,7 +33,8 @@ import ( 5. Determine if the request / response ended with an error 6. Extract response values from HTTP response to Values struct 6.1. Determine the response status that was sent to the client `v.HTTPResponseStatusCode = ?` - 6.2. Extract response body size `v.HTTPResponseBodySize = ?` + 6.2. Determine the error type if the request ended with an error `v.ErrorType = ErrorType(status, err)` + 6.3. Extract response body size `v.HTTPResponseBodySize = ?` 7. Attributes from response can be acquired from extracted values with `attr := v.SpanEndAttributes()` 7.1. If your middleware needs to support additional end attributes, add them to attributes. 8. Create a RecordValues struct for metrics recording `iv := RecordValues{...}`. @@ -111,7 +112,9 @@ func (m *Metrics) Record(ctx context.Context, v RecordValues) { o := metric.WithAttributeSet(attribute.NewSet(attrs...)) m.requestDurationHistogram.Inst().Record(ctx, v.RequestDuration.Seconds(), o) - m.requestBodySizeHistogram.Inst().Record(ctx, v.ExtractedValues.HTTPRequestBodySize, o) + if v.ExtractedValues.HTTPRequestBodySize >= 0 { // -1 means unknown size (chunked or streamed body) + m.requestBodySizeHistogram.Inst().Record(ctx, v.ExtractedValues.HTTPRequestBodySize, o) + } m.responseBodySizeHistogram.Inst().Record(ctx, v.ExtractedValues.HTTPResponseBodySize, o) } @@ -148,32 +151,34 @@ type Values struct { // // Requirement Level: // * span - conditionally required if raw value differs from `http.request.method` (different case) or `http.request.method` is `_OTHER` - // * metric - opt in, same rules as span - HTTPMethodOriginal string // metric, span + // * metric - not used, as the value is chosen by the client and would make metric cardinality unbounded + HTTPMethodOriginal string // span // ServerAddress (`server.address`) is the Name of the local HTTP server that received the request. - // This value can be provided by middleware configuration or extracted from `Request.Host`. + // This value can be provided by middleware configuration or extracted from `Request.Host` (together with ServerPort). // Example values: `example.com` `10.1.2.80`, `/tmp/my.sock` // See also: https://opentelemetry.io/docs/specs/semconv/http/http-spans/#setting-serveraddress-and-serverport-attributes // Spec: https://opentelemetry.io/docs/specs/semconv/registry/attributes/server/ // // Requirement Level: // * span - Recommended - // * metric - Opt-In - ServerAddress string // metric, span + // * metric - Opt-In, not added by MetricAttributes as `Request.Host` is chosen by the client. Add it with a custom + // metric attributes function when the value comes from configuration. + ServerAddress string // span // ServerPort (`server.port`) is the Port of the local HTTP server that received the request. - // This value can be provided by middleware configuration or extracted from `Request.Host`. + // This value can be provided by middleware configuration or extracted from `Request.Host` (only when ServerAddress + // is extracted from it too). // Example values: `80` `8080`, `443` // See also: https://opentelemetry.io/docs/specs/semconv/http/http-spans/#setting-serveraddress-and-serverport-attributes // Spec: https://opentelemetry.io/docs/specs/semconv/registry/attributes/server/ // // Requirement Level: // * span - conditionally Required if available and `server.address` is set - // * metric - Opt-In - ServerPort int // metric, span + // * metric - Opt-In, not added by MetricAttributes, same as ServerAddress + ServerPort int // span - // NetworkPeerAdress (`network.peer.address`) is peer address of the network connection - IP address or Unix domain socket name. + // NetworkPeerAddress (`network.peer.address`) is peer address of the network connection - IP address or Unix domain socket name. // Spec: https://opentelemetry.io/docs/specs/semconv/registry/attributes/network/ // // Go: This value is derived from `Request.RemoteAddr` field value. @@ -198,8 +203,8 @@ type Values struct { // Spec: https://opentelemetry.io/docs/specs/semconv/registry/attributes/client/ // // Go: This value is derived by default from `Request.RemoteAddr` field value and is same as `network.peer.address`. - // Middleware creators or users can override with `Request.Header.Get("X-Forwarded-For")` value but be warned - // that it is very easy to spoof HTTP headers. + // The Echo middleware sets it from `Context.RealIP()`, which follows the Echo instance IP extraction settings + // (`Echo.IPExtractor`); headers like `X-Forwarded-For` are easy to spoof when they are trusted from any client. // // Requirement Level: // * span - Recommended if `network.peer.address` is set. @@ -268,7 +273,8 @@ type Values struct { // all static path segments, with dynamic path segments represented with placeholders. // Spec: https://opentelemetry.io/docs/specs/semconv/registry/attributes/http/ // - // Go: This value is taken from `Request.Pattern` field. + // Go: The Echo middleware takes this value from `Context.Path()`, the matched Echo route. ExtractRequest uses the + // `Request.Pattern` field when the value is not set. // // Requirement Level: // * span - Recommended @@ -278,23 +284,35 @@ type Values struct { // HTTPRequestBodySize (`http.request.body.size`) is the size of the request payload body in bytes. This is the number // of bytes transferred excluding headers and is often, but not always, present as the [Content-Length](https://www.rfc-editor.org/rfc/rfc9110.html#field.content-length) // header. For requests using transport encoding, this should be the compressed size. - // Spec: https://opentelemetry.io/docs/specs/semconv/http/http-metrics/#metric-httpclientrequestbodysize + // Spec: https://opentelemetry.io/docs/specs/semconv/http/http-metrics/#metric-httpserverrequestbodysize // - // Go: This value is taken from `Request.ContentLength` can be negative (-1) if the size is unknown. + // Go: This value is taken from `Request.ContentLength` and is negative (-1) if the size is unknown. Unknown size is + // not recorded. // // Requirement Level: - // * span - opt-in attribute - // * metric - optional, is actual Histogram metric (`http.client.request.body.size`) and NOT attribute to metric. + // * span - Opt-In, not added by SpanEndAttributes. Add it with a span end attributes function if needed. + // * metric - optional, is actual Histogram metric (`http.server.request.body.size`) and NOT attribute to metric. HTTPRequestBodySize int64 // metric // HTTPResponseStatusCode (`http.response.status_code`) is HTTP response status code. // See also RFC: https://datatracker.ietf.org/doc/html/rfc7231#section-6 // Spec: https://opentelemetry.io/docs/specs/semconv/registry/attributes/http/ // + // Zero means no status was sent (for example the handler aborted the request); the attribute is not set then. + // // Requirement Level: - // * span - opt-in attribute + // * span - conditionally Required if and only if one was received/sent // * metric - conditionally Required if and only if one was received/sent - HTTPResponseStatusCode int // metric + HTTPResponseStatusCode int // metric, span + + // ErrorType (`error.type`) describes the class of error the request ended with. It is empty when the request did + // not end with an error. Use ErrorType function to determine the value. + // Spec: https://opentelemetry.io/docs/specs/semconv/registry/attributes/error/ + // + // Requirement Level: + // * span - conditionally Required if request has ended with an error + // * metric - conditionally Required if request has ended with an error + ErrorType string // metric, span // HTTPResponseBodySize (`http.response.body.size`) is the size of the response payload body in bytes. This is // the number of bytes transferred excluding headers and is often, but not always, present as the @@ -303,7 +321,7 @@ type Values struct { // Spec: https://opentelemetry.io/docs/specs/semconv/http/http-metrics/#metric-httpserverresponsebodysize // // Requirement Level: - // * span - opt-in attribute + // * span - Opt-In, not added by SpanEndAttributes. Add it with a span end attributes function if needed. // * metric - optional, is actual Histogram metric (`http.server.response.body.size`) and NOT attribute to metric. HTTPResponseBodySize int64 // metric } @@ -312,14 +330,12 @@ type Values struct { func (v *Values) ExtractRequest(r *http.Request) error { var errs []error - if v.ServerAddress == "" || v.ServerPort == 0 { + if v.ServerAddress == "" { // take address and port from the same source, do not mix configuration and Request.Host host, port, err := SplitAddress(r.Host) if err != nil { errs = append(errs, fmt.Errorf("failed to split Request.Host: %w", err)) } - if v.ServerAddress == "" { - v.ServerAddress = host - } + v.ServerAddress = host if v.ServerPort == 0 { v.ServerPort = port } @@ -371,6 +387,15 @@ func (v *Values) SpanStartAttributes() []attribute.KeyValue { // Use this method instead of `SpanStartAttributes()` when you want to preallocate/reuse attributes slice. func (v *Values) AppendStartAttributes(attrs []attribute.KeyValue) []attribute.KeyValue { attrs = v.appendCommonAttributes(attrs) + if v.ServerAddress != "" { + attrs = append(attrs, semconv.ServerAddress(v.ServerAddress)) + if v.ServerPort != 0 { + attrs = append(attrs, semconv.ServerPort(v.ServerPort)) + } + } + if v.HTTPMethodOriginal != "" { + attrs = append(attrs, semconv.HTTPRequestMethodOriginal(v.HTTPMethodOriginal)) + } if v.NetworkPeerAddress != "" { attrs = append(attrs, semconv.NetworkPeerAddress(v.NetworkPeerAddress)) } @@ -390,24 +415,34 @@ func (v *Values) AppendStartAttributes(attrs []attribute.KeyValue) []attribute.K } // SpanEndAttributes returns a list of attributes to be used when ending a span, after the next handler has been executed. +// +// It adds `http.response.status_code` (if a status was sent) and `error.type` (if the request ended with an error). +// The Opt-In `http.request.body.size` and `http.response.body.size` attributes are not added. func (v *Values) SpanEndAttributes() []attribute.KeyValue { - return v.AppendSpanEndAttributes(make([]attribute.KeyValue, 0, 3)) + return v.AppendSpanEndAttributes(make([]attribute.KeyValue, 0, 2)) } // AppendSpanEndAttributes appends attributes to be used when ending a span, after the next handler has been executed. // Use this method instead of `SpanEndAttributes()` when you want to preallocate/reuse attributes slice. func (v *Values) AppendSpanEndAttributes(attrs []attribute.KeyValue) []attribute.KeyValue { - return append(attrs, - semconv.HTTPResponseStatusCode(v.HTTPResponseStatusCode), - semconv.HTTPRequestBodySize(int(v.HTTPRequestBodySize)), - semconv.HTTPResponseBodySize(int(v.HTTPResponseBodySize)), - ) + if v.HTTPResponseStatusCode != 0 { + attrs = append(attrs, semconv.HTTPResponseStatusCode(v.HTTPResponseStatusCode)) + } + if v.ErrorType != "" { + attrs = append(attrs, semconv.ErrorTypeKey.String(v.ErrorType)) + } + return attrs } // MetricAttributes creates attributes for metric instruments from extracted values. +// +// Only low cardinality attributes are included: `http.request.method`, `url.scheme`, `http.route`, +// `network.protocol.name`, `network.protocol.version`, `http.response.status_code` and `error.type`. Opt-In attributes +// whose values the client chooses (`server.address` and `server.port` from `Request.Host`, +// `http.request.method_original`) are not included. // See also: https://opentelemetry.io/docs/specs/semconv/http/http-metrics/ func (v *Values) MetricAttributes() []attribute.KeyValue { - return v.AppendMetricAttributes(make([]attribute.KeyValue, 0, 8+1)) + return v.AppendMetricAttributes(make([]attribute.KeyValue, 0, 8+2)) } // AppendMetricAttributes appends attributes for metric instruments from extracted values. @@ -418,6 +453,9 @@ func (v *Values) AppendMetricAttributes(attrs []attribute.KeyValue) []attribute. if v.HTTPResponseStatusCode != 0 { attrs = append(attrs, semconv.HTTPResponseStatusCode(v.HTTPResponseStatusCode)) } + if v.ErrorType != "" { + attrs = append(attrs, semconv.ErrorTypeKey.String(v.ErrorType)) + } return attrs } @@ -428,18 +466,11 @@ func (v *Values) appendCommonAttributes(attrs []attribute.KeyValue) []attribute. } attrs = append(attrs, method, - semconv.ServerAddress(v.ServerAddress), semconv.URLScheme(v.URLScheme), ) if v.HTTPRoute != "" { attrs = append(attrs, semconv.HTTPRoute(v.HTTPRoute)) } - if v.ServerPort != 0 { - attrs = append(attrs, semconv.ServerPort(v.ServerPort)) - } - if v.HTTPMethodOriginal != "" { - attrs = append(attrs, semconv.HTTPRequestMethodOriginal(v.HTTPMethodOriginal)) - } if v.NetworkProtocolName != "" { attrs = append(attrs, semconv.NetworkProtocolName(v.NetworkProtocolName)) } @@ -454,7 +485,7 @@ func (v *Values) appendCommonAttributes(attrs []attribute.KeyValue) []attribute. // the PATCH method defined in [RFC5789](https://www.rfc-editor.org/rfc/rfc5789.html) and the // QUERY method defined in [httpbis-safe-method-w-body](https://datatracker.ietf.org/doc/draft-ietf-httpbis-safe-method-w-body/?include_text=1). // -// Source: OpenTelemetry semantic conventions 1.39.0 +// Source: OpenTelemetry semantic conventions 1.40.0 var knownMethods = map[string]attribute.KeyValue{ http.MethodConnect: semconv.HTTPRequestMethodConnect, http.MethodDelete: semconv.HTTPRequestMethodDelete, @@ -502,6 +533,13 @@ func splitProto(proto string) (name string, version string) { default: name = strings.ToLower(name) } + // Go reports HTTP/2 and HTTP/3 as "2.0" and "3.0"; the semantic conventions use "2" and "3". + switch version { + case "2.0": + version = "2" + case "3.0": + version = "3" + } return name, version } @@ -596,3 +634,59 @@ func SpanStatus(code int, err error) (codes.Code, string) { } return codes.Unset, "" } + +// statusCoder is implemented by errors that carry an HTTP status code, for example Echo v5's HTTPError. +type statusCoder interface { + StatusCode() int +} + +// ErrorType returns the `error.type` attribute value for a server span and metrics, based on the resolved HTTP +// response status code and the error that occurred while handling the request. It returns an empty string when the +// request did not end with an error. +// +// The request is considered to have ended with an error under the same rules as SpanStatus: a 5xx status code, an +// invalid status code, or an error alongside a 1xx-3xx status code. A 4xx status code is not an error for server spans. +// +// The returned value is: +// - the status code as a string (for example "500") for a 5xx status code when there is no error or the error +// carries an HTTP status code (implements `StatusCode() int`, like Echo v5's HTTPError), +// - the error type for other errors, as returned by semconv.ErrorType: the value of an `ErrorType() string` method +// found in the error chain, otherwise the Go type of the error after unwrapping errors created with +// `fmt.Errorf("...: %w", err)` (for example "*net.OpError"). Errors joining several errors (`errors.Join`, +// `fmt.Errorf` with more than one `%w`) are not unwrapped and report their own type, +// - the status code as a string for a status code above 599 without an error (net/http sends codes up to 999), +// - "_OTHER" for a status code below 100 without an error. +// +// The spec requires low cardinality. Error types are bounded by the code base, unless errors implement +// `ErrorType() string` with values that vary per request; avoid that, as error.type is also a metric attribute. +// +// Spec: +// +// If the request fails with an error before response status code was sent or received, error.type SHOULD be set to +// exception type (its fully-qualified class name, if applicable) or a component-specific low cardinality error +// identifier. +// +// If response status code was sent or received and status indicates an error according to HTTP span status +// definition, error.type SHOULD be set to the status code number (represented as a string), an exception type (if +// thrown) or a component-specific error identifier. +// +// Reference: +// - [HTTP server span](https://opentelemetry.io/docs/specs/semconv/http/http-spans/#http-server-span) +// - [error.type](https://opentelemetry.io/docs/specs/semconv/registry/attributes/error/) +func ErrorType(code int, err error) string { + validCode := code >= 100 && code < 600 + switch { + case validCode && code >= 400 && code < 500: + return "" // this instrumentation creates server spans, for those a 4xx response is not an error + case validCode && code < 400 && err == nil: + return "" + case validCode && code >= 500 && (err == nil || errors.As(err, new(statusCoder))): + return strconv.Itoa(code) + case err != nil: + return semconv.ErrorType(err).Value.AsString() + case code >= 600 && code <= 999: + return strconv.Itoa(code) // invalid for the spec, but net/http sends it + default: + return semconv.ErrorTypeOther.Value.AsString() + } +} diff --git a/extrator_test.go b/extractor_test.go similarity index 81% rename from extrator_test.go rename to extractor_test.go index 4dc050f..38f26f0 100644 --- a/extrator_test.go +++ b/extractor_test.go @@ -3,6 +3,8 @@ package echootel import ( "context" "errors" + "fmt" + "net" "net/http" "testing" @@ -200,8 +202,29 @@ func TestValues_SpanEndAttributes(t *testing.T) { }, expectAttributes: []attribute.KeyValue{ attribute.Int("http.response.status_code", 200), - attribute.Int("http.request.body.size", 999), - attribute.Int("http.response.body.size", 1000), + }, + }, + { + name: "no status sent", + given: Values{ + HTTPResponseStatusCode: 0, + ErrorType: "panic", + }, + expectAttributes: []attribute.KeyValue{ + attribute.String("error.type", "panic"), + }, + }, + { + name: "request ended with an error", + given: Values{ + HTTPRequestBodySize: 0, + HTTPResponseBodySize: 21, + HTTPResponseStatusCode: 500, + ErrorType: "500", + }, + expectAttributes: []attribute.KeyValue{ + attribute.Int("http.response.status_code", 500), + attribute.String("error.type", "500"), }, }, } @@ -234,9 +257,7 @@ func TestValues_MetricAttributes(t *testing.T) { }, expectAttributes: []attribute.KeyValue{ attribute.String("http.request.method", "GET"), - attribute.String("http.request.method_original", "gEt"), attribute.String("url.scheme", "http"), - attribute.String("server.address", "example.com"), attribute.String("network.protocol.name", "http"), attribute.String("network.protocol.version", "1.1"), attribute.Int64("http.response.status_code", 200), @@ -257,14 +278,28 @@ func TestValues_MetricAttributes(t *testing.T) { expectAttributes: []attribute.KeyValue{ attribute.String("http.request.method", "GET"), attribute.String("url.scheme", "http"), - attribute.String("server.address", "example.com"), - attribute.Int("server.port", 9999), attribute.String("network.protocol.name", "http"), attribute.String("network.protocol.version", "1.1"), attribute.Int64("http.response.status_code", 200), attribute.String("http.route", "/path/${id}"), }, }, + { + name: "request ended with an error", + given: Values{ + HTTPMethod: "GET", + ServerAddress: "example.com", + URLScheme: "http", + HTTPResponseStatusCode: 503, + ErrorType: "503", + }, + expectAttributes: []attribute.KeyValue{ + attribute.String("http.request.method", "GET"), + attribute.String("url.scheme", "http"), + attribute.Int64("http.response.status_code", 503), + attribute.String("error.type", "503"), + }, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -621,6 +656,18 @@ func TestSplitProto(t *testing.T) { expectName: "http", expectVersion: "1.1", }, + { + name: "HTTP/2.0 is reported as 2", + whenProto: "HTTP/2.0", + expectName: "http", + expectVersion: "2", + }, + { + name: "HTTP/3.0 is reported as 3", + whenProto: "HTTP/3.0", + expectName: "http", + expectVersion: "3", + }, { name: "quic uppercase", whenProto: "QUIC/2", @@ -789,3 +836,124 @@ func TestSpanStatus(t *testing.T) { }) } } + +// testStatusError carries an HTTP status code, like Echo's HTTPError. +type testStatusError struct { + code int +} + +func (e *testStatusError) Error() string { return fmt.Sprintf("status %d", e.code) } +func (e *testStatusError) StatusCode() int { return e.code } + +func TestErrorType(t *testing.T) { + var testCases = []struct { + name string + whenStatus int + whenError error + expect string + }{ + { + name: "success status 200", + whenStatus: 200, + }, + { + name: "redirect status 302", + whenStatus: 302, + }, + { + name: "client error 404 is not an error for server span", + whenStatus: 404, + }, + { + name: "client error 400 with error is not an error for server span", + whenStatus: 400, + whenError: &testStatusError{code: 400}, + }, + { + name: "server error 500 without error value", + whenStatus: 500, + expect: "500", + }, + { + name: "server error 503 with error carrying status code", + whenStatus: 503, + whenError: &testStatusError{code: 503}, + expect: "503", + }, + { + name: "server error 500 with wrapped error carrying status code", + whenStatus: 500, + whenError: fmt.Errorf("handler: %w", &testStatusError{code: 500}), + expect: "500", + }, + { + name: "server error 500 with plain error uses error type", + whenStatus: 500, + whenError: errors.New("something failed"), + expect: "*errors.errorString", + }, + { + name: "server error 500 with fmt.Errorf wrapped error reports the wrapped error type", + whenStatus: 500, + whenError: fmt.Errorf("db: %w", fmt.Errorf("query: %w", &net.OpError{Op: "read", Err: errors.New("reset")})), + expect: "*net.OpError", + }, + { + name: "server error 500 with errors.Join reports the join type", + whenStatus: 500, + whenError: errors.Join(errors.New("a"), errors.New("b")), + expect: "*errors.joinError", + }, + { + name: "server error 500 with fmt.Errorf wrapping two errors reports the wrapper type", + whenStatus: 500, + whenError: fmt.Errorf("%w and %w", errors.New("a"), errors.New("b")), + expect: "*fmt.wrapErrors", + }, + { + name: "server error 500 with ErrorType method in the error chain", + whenStatus: 500, + whenError: fmt.Errorf("handler: %w", testTypedError{}), + expect: "test.custom", + }, + { + name: "error alongside success status uses error type", + whenStatus: 200, + whenError: errors.New("network error"), + expect: "*errors.errorString", + }, + { + name: "invalid status code with error uses error type", + whenStatus: 0, + whenError: errors.New("network error"), + expect: "*errors.errorString", + }, + { + name: "status code above 599 without error reports the code", + whenStatus: 799, + expect: "799", + }, + { + name: "status code below 100 without error", + whenStatus: 99, + expect: "_OTHER", + }, + { + name: "status code above 999 without error", + whenStatus: 1000, + expect: "_OTHER", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expect, ErrorType(tc.whenStatus, tc.whenError)) + }) + } +} + +// testTypedError implements ErrorType() string, which semconv.ErrorType prefers over the Go type. +type testTypedError struct{} + +func (testTypedError) Error() string { return "typed" } +func (testTypedError) ErrorType() string { return "test.custom" } diff --git a/go.mod b/go.mod index e1dd470..eb97c58 100644 --- a/go.mod +++ b/go.mod @@ -1,27 +1,25 @@ -module github.com/labstack/echo-opentelemetry +module github.com/labstack/echo-otel/v5 go 1.25.0 require ( github.com/labstack/echo/v5 v5.2.1 - github.com/stretchr/testify v1.11.1 - go.opentelemetry.io/contrib/propagators/b3 v1.42.0 - go.opentelemetry.io/otel v1.44.0 - go.opentelemetry.io/otel/metric v1.44.0 - go.opentelemetry.io/otel/sdk v1.42.0 - go.opentelemetry.io/otel/sdk/metric v1.42.0 - go.opentelemetry.io/otel/trace v1.44.0 + github.com/stretchr/testify v1.12.1 + go.opentelemetry.io/contrib/propagators/b3 v1.46.0 + go.opentelemetry.io/otel v1.46.0 + go.opentelemetry.io/otel/metric v1.46.0 + go.opentelemetry.io/otel/sdk v1.46.0 + go.opentelemetry.io/otel/sdk/metric v1.46.0 + go.opentelemetry.io/otel/trace v1.46.0 ) require ( github.com/cespare/xxhash/v2 v2.3.0 // indirect - github.com/davecgh/go-spew v1.1.1 // indirect - github.com/go-logr/logr v1.4.3 // indirect + github.com/go-logr/logr v1.4.4 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/google/uuid v1.6.0 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect go.opentelemetry.io/auto/sdk v1.2.1 // indirect - golang.org/x/sys v0.42.0 // indirect + go.yaml.in/yaml/v3 v3.0.5 // indirect + golang.org/x/sys v0.47.0 // indirect golang.org/x/time v0.15.0 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 8254820..0555e6c 100644 --- a/go.sum +++ b/go.sum @@ -1,54 +1,43 @@ github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= -github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= -github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= -github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-logr/logr v1.4.4 h1:tG4xh9yMsRCAiodLVTxyrkzSZ9+o0L1Kg/+cPVcbP/8= +github.com/go-logr/logr v1.4.4/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= -github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= -github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= -github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/labstack/echo/v5 v5.2.1 h1:TzpIksY6zLMzV0T0ycYbvTEoj9w6o6AcL5twg182VTY= github.com/labstack/echo/v5 v5.2.1/go.mod h1:SyvlSdObGjRXeQfCCXW/sybkZdOOQZBmpKF0bvALaeo= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= -github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= -github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= -github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= -github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE= +github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg= go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= -go.opentelemetry.io/contrib/propagators/b3 v1.42.0 h1:B2Pew5ufEtgkjLF+tSkXjgYZXQr9m7aCm1wLKB0URbU= -go.opentelemetry.io/contrib/propagators/b3 v1.42.0/go.mod h1:iPgUcSEF5DORW6+yNbdw/YevUy+QqJ508ncjhrRSCjc= -go.opentelemetry.io/otel v1.44.0 h1:JjwHmHpA4iZ3wBxluu2fbbE7j4kqlE8jXyAyPXH7HqU= -go.opentelemetry.io/otel v1.44.0/go.mod h1:BMgjTHL9WPRlRjL2oZCBTL4whCGtXch2H4BhOPIAyYc= -go.opentelemetry.io/otel/metric v1.44.0 h1:1w0gILTcHdr3YI+ixLyjemwrVnsMURbTZFrSYCdDdmc= -go.opentelemetry.io/otel/metric v1.44.0/go.mod h1:8O7hanEPBNgEMmybD3s2VBKcgWOCsA6tzHBPODAiquo= -go.opentelemetry.io/otel/sdk v1.42.0 h1:LyC8+jqk6UJwdrI/8VydAq/hvkFKNHZVIWuslJXYsDo= -go.opentelemetry.io/otel/sdk v1.42.0/go.mod h1:rGHCAxd9DAph0joO4W6OPwxjNTYWghRWmkHuGbayMts= -go.opentelemetry.io/otel/sdk/metric v1.42.0 h1:D/1QR46Clz6ajyZ3G8SgNlTJKBdGp84q9RKCAZ3YGuA= -go.opentelemetry.io/otel/sdk/metric v1.42.0/go.mod h1:Ua6AAlDKdZ7tdvaQKfSmnFTdHx37+J4ba8MwVCYM5hc= -go.opentelemetry.io/otel/trace v1.44.0 h1:jxF5CsGYCe74MCRx2X4g7WsY/VBKRqqpNvXlX/6gtIk= -go.opentelemetry.io/otel/trace v1.44.0/go.mod h1:oLl1jrMQAVo6v3GAggN+1VH9VIz9iUSvW53sW1Q8PIE= +go.opentelemetry.io/contrib/propagators/b3 v1.46.0 h1:OFVqWObn7xLIbOjE/koO0LS9fZJNgAyBD0msA+UQAoc= +go.opentelemetry.io/contrib/propagators/b3 v1.46.0/go.mod h1:t/d64xy7xuuEDJN/4ThqohLgRhIuQxL9y7P1v02bYuM= +go.opentelemetry.io/otel v1.46.0 h1:FHt5/CDyVxi/8IM1CH7VE/rRgq3kLHa2mSTVMO8AWyc= +go.opentelemetry.io/otel v1.46.0/go.mod h1:Gj3SEScelsNC45tp4nSxRYlS+f5iez7W8XPMCt905kE= +go.opentelemetry.io/otel/metric v1.46.0 h1:yBnkXvgV7AXFILZc5K6IZe/CBFF3OS7BJ8ov6/lj0K8= +go.opentelemetry.io/otel/metric v1.46.0/go.mod h1:iPmdWqifKUdzziPkvvzIJXITl56fQx2mGM/DHLB3/2o= +go.opentelemetry.io/otel/metric/x v0.68.0 h1:TA/cBT23D3MnxYPwHL7YFOdYGdx0A0v+s7Mzotpd1dU= +go.opentelemetry.io/otel/metric/x v0.68.0/go.mod h1:agudOmvWhwUTjgibWDzxD2PoWYnpw5Ht5jISYOD2Hd4= +go.opentelemetry.io/otel/sdk v1.46.0 h1:h5CNQQjEbuQXY/JfZtgt3i7HVFV3aHPO2OAwO2eTYPI= +go.opentelemetry.io/otel/sdk v1.46.0/go.mod h1:GAERFXFt5SYCEB+YiKUbMBeza6UaDH7GmGOZEfh2gSM= +go.opentelemetry.io/otel/sdk/metric v1.46.0 h1:0piZ26EG4RBfebb2jhDH6ERCYHoVWduc3kLgPCwSnSE= +go.opentelemetry.io/otel/sdk/metric v1.46.0/go.mod h1:I1PbKrdVc8Qu8HYVDNtqVIwLwjNrhsV/uFuxfwg8mO4= +go.opentelemetry.io/otel/trace v1.46.0 h1:OULy7ccdJnZtJ0UDYFOIGaCmiWzJ8Vi2G/Rsu60qs1c= +go.opentelemetry.io/otel/trace v1.46.0/go.mod h1:J7GAXweO77XSFkB/rmAqk9D6ihszhFjLU+d9WuUxDLI= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= +go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= +go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg= golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= -golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= -golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/text v0.33.0 h1:B3njUFyqtHDUI5jMn1YIr5B0IE2U0qck04r6d4KPAxE= golang.org/x/text v0.33.0/go.mod h1:LuMebE6+rBincTi9+xWTY8TztLzKHc/9C1uBCG27+q8= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= -gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= -gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= -gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/otel.go b/otel.go index bd90e01..da5f398 100644 --- a/otel.go +++ b/otel.go @@ -4,16 +4,19 @@ package echootel import ( + "errors" "fmt" + "net/http" "time" "github.com/labstack/echo/v5" "github.com/labstack/echo/v5/middleware" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/propagation" - semconv "go.opentelemetry.io/otel/semconv/v1.39.0" + semconv "go.opentelemetry.io/otel/semconv/v1.40.0" oteltrace "go.opentelemetry.io/otel/trace" ) @@ -21,13 +24,14 @@ const ( // TracerKey is the key used to store the tracer in the echo context. TracerKey = "labstack-echo-otelecho-tracer" // ScopeName is the instrumentation scope name. - ScopeName = "github.com/labstack/echo-opentelemetry" + ScopeName = "github.com/labstack/echo-otel/v5" ) // Config is used to configure the middleware. type Config struct { - // ServerName is set as `server.address` and `server.port` for span and metrics attributes. - // Example: "api.example.com" or "example.com:8080" + // ServerName is set as `server.address` and `server.port` span attributes. They are not added to metrics by + // default (Opt-In in the semantic conventions); add them with MetricAttributes if needed. + // Example: "api.example.com" or "example.com:8080". Without a port, `server.port` is not set. // // If known, this value must be set to the server’s canonical (primary) name. // For example, in Apache this corresponds to the ServerName directive @@ -42,7 +46,7 @@ type Config struct { // // If the primary server name is unknown, this field should be set to an // empty string. In that case, Request.Host will be used to resolve the - // effective server name and port. + // effective server name and port. Request.Host is chosen by the client. ServerName string // Skipper defines a function to skip middleware. @@ -130,6 +134,9 @@ func (config Config) ToMiddleware() (echo.MiddlewareFunc, error) { if sErr != nil { return nil, fmt.Errorf("otel middleware failed to parse server name: %w", sErr) } + if host == "" { + return nil, fmt.Errorf("otel middleware failed to parse server name: %q has no host", config.ServerName) + } serverHost = host serverPort = port } @@ -177,6 +184,9 @@ func (config Config) ToMiddleware() (echo.MiddlewareFunc, error) { config.OnExtractionError(c, err) } } + // The route comes from Echo only. Before routing (middleware added with Echo.Pre) it is empty, and + // Request.Pattern may hold the pattern of an outer http.ServeMux, which is not the Echo route. + ev.HTTPRoute = c.Path() spanAttributes := ev.SpanStartAttributes() if config.SpanStartAttributes != nil { spanAttributes = config.SpanStartAttributes(c, &ev, spanAttributes) @@ -212,43 +222,109 @@ func (config Config) ToMiddleware() (echo.MiddlewareFunc, error) { } }() - err := next(c) + // end records the span status, span end attributes and metrics. statusSent is false when no response + // status was sent to the client (the handler aborted the request). + end := func(err error, statusSent bool) { + if ev.HTTPRoute == "" { + // middleware added with Echo.Pre runs before routing, so the route is known only after the next handler + if route := c.Path(); route != "" { + ev.HTTPRoute = route + span.SetName(SpanNameFormatter(ev)) + span.SetAttributes(semconv.HTTPRoute(route)) + } + } - if err != nil { - span.SetAttributes(semconv.ErrorType(err)) - if config.OnNextError != nil { - config.OnNextError(c, err) + resp, status := echo.ResolveResponseStatus(c.Response(), err) + if !statusSent { + status = 0 // http.response.status_code is set only if a status was sent } - } - resp, status := echo.ResolveResponseStatus(c.Response(), err) - span.SetStatus(SpanStatus(status, err)) + ev.HTTPResponseStatusCode = status + + var pErr *panicError + if errors.As(err, &pErr) || status == 0 { + // a panic is an error regardless of the status code that was already sent (even 4xx) + span.SetStatus(codes.Error, err.Error()) + ev.ErrorType = ErrorType(0, err) + } else { + span.SetStatus(SpanStatus(status, err)) + ev.ErrorType = ErrorType(status, err) + } + if resp != nil { + ev.HTTPResponseBodySize = resp.Size + } + endAttributes := ev.SpanEndAttributes() + if config.SpanEndAttributes != nil { + endAttributes = config.SpanEndAttributes(c, &ev, endAttributes) + } + span.SetAttributes(endAttributes...) - ev.HTTPResponseStatusCode = status - if resp != nil { - ev.HTTPResponseBodySize = resp.Size - } - endAttributes := ev.SpanEndAttributes() - if config.SpanEndAttributes != nil { - endAttributes = config.SpanEndAttributes(c, &ev, endAttributes) + iv := RecordValues{ + RequestDuration: time.Since(requestStartTime), + ExtractedValues: ev, + Attributes: nil, + // when Attributes are left nil, they are extracted with `iv.ExtractedValues.MetricAttributes()` inside `metrics.Record()` call + } + if config.MetricAttributes != nil { + iv.Attributes = config.MetricAttributes(c, &ev) + } + metrics.Record(c, iv) } - span.SetAttributes(endAttributes...) - iv := RecordValues{ - RequestDuration: time.Since(requestStartTime), - ExtractedValues: ev, - Attributes: nil, - // when Attributes are left nil, they are extracted with `iv.ExtractedValues.MetricAttributes()` inside `metrics.Record()` call + completed := false + defer func() { + if completed { + return + } + // The next handler (or OnNextError) panicked or called runtime.Goexit. Record the request as failed, then + // re-panic so that a Recover middleware added before this one (or http.Server) still handles the panic. + r := recover() + committed := false + if resp, uErr := echo.UnwrapResponse(c.Response()); uErr == nil { + committed = resp.Committed + } + if r == nil { // runtime.Goexit + end(&panicError{value: "runtime.Goexit"}, committed) + return + } + defer panic(r) // re-panic with the original value, even if recording panics + // http.ErrAbortHandler aborts the response: no status is sent unless the response was already committed + end(&panicError{value: r}, committed || r != http.ErrAbortHandler) + }() + + err := next(c) + if err != nil && config.OnNextError != nil { + config.OnNextError(c, err) } - if config.MetricAttributes != nil { - iv.Attributes = config.MetricAttributes(c, &ev) + completed = true + + recordErr := err + var panicStackErr *middleware.PanicStackError + if errors.As(err, &panicStackErr) { + // Recover middleware added after this one turned a panic into an error + recordErr = &panicError{value: panicStackErr.Err} } - metrics.Record(c, iv) + end(recordErr, true) return err } }, nil } +// panicError is the error recorded for a request whose handler panicked. Its error type is "panic". +type panicError struct { + value any +} + +func (e *panicError) Error() string { return fmt.Sprintf("panic: %v", e.value) } +func (e *panicError) ErrorType() string { return "panic" } + +// Unwrap returns the panic value if it is an error, so that a panic value carrying a status code (for example +// echo.ErrUnauthorized) resolves to the status code the error handler sends for it. +func (e *panicError) Unwrap() error { + err, _ := e.value.(error) + return err +} + // echoMetricsRecorder is the default implementation for Echo metric recording interface type echoMetricsRecorder struct { *Metrics diff --git a/otel_behavior_test.go b/otel_behavior_test.go new file mode 100644 index 0000000..4650503 --- /dev/null +++ b/otel_behavior_test.go @@ -0,0 +1,469 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: © 2026 LabStack and Echo contributors + +package echootel + +import ( + "bytes" + "errors" + "io" + "mime/multipart" + "net/http" + "net/http/httptest" + "os" + "testing" + + "github.com/labstack/echo/v5" + "github.com/labstack/echo/v5/middleware" + "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + metricnoop "go.opentelemetry.io/otel/metric/noop" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/metric/metricdata" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" + "go.opentelemetry.io/otel/trace" + tracenoop "go.opentelemetry.io/otel/trace/noop" +) + +func newTestProviders() (*tracetest.InMemoryExporter, *sdktrace.TracerProvider, *sdkmetric.ManualReader, *sdkmetric.MeterProvider) { + exporter := tracetest.NewInMemoryExporter() + tp := sdktrace.NewTracerProvider(sdktrace.WithSyncer(exporter)) + reader := sdkmetric.NewManualReader() + mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + return exporter, tp, reader, mp +} + +// durationPoints returns the attribute sets of the http.server.request.duration data points. +func durationPoints(t *testing.T, reader *sdkmetric.ManualReader) []attribute.Set { + t.Helper() + rm := metricdata.ResourceMetrics{} + assert.NoError(t, reader.Collect(t.Context(), &rm)) + var sets []attribute.Set + for _, sm := range rm.ScopeMetrics { + for _, m := range sm.Metrics { + if m.Name != "http.server.request.duration" { + continue + } + for _, dp := range m.Data.(metricdata.Histogram[float64]).DataPoints { + sets = append(sets, dp.Attributes) + } + } + } + return sets +} + +func metricHas(set attribute.Set, key attribute.Key) bool { + _, ok := set.Value(key) + return ok +} + +func TestPanicIsRecordedAndRepanicked(t *testing.T) { + exporter, tp, reader, mp := newTestProviders() + + e := echo.New() + e.Use(middleware.Recover()) // usual order: Recover wraps the otel middleware + e.Use(NewMiddlewareWithConfig(Config{ServerName: "foobar", TracerProvider: tp, MeterProvider: mp})) + e.GET("/panic", func(c *echo.Context) error { + panic("boom") + }) + + w := httptest.NewRecorder() + e.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/panic", http.NoBody)) + + assert.Equal(t, http.StatusInternalServerError, w.Result().StatusCode, "Recover must still handle the panic") + spans := exporter.GetSpans() + if !assert.Len(t, spans, 1) { + return + } + assert.Equal(t, codes.Error, spans[0].Status.Code) + assert.Equal(t, "panic: boom", spans[0].Status.Description) + assert.Contains(t, spans[0].Attributes, attribute.Int("http.response.status_code", http.StatusInternalServerError)) + assert.Contains(t, spans[0].Attributes, attribute.String("error.type", "panic")) + + points := durationPoints(t, reader) + if assert.Len(t, points, 1) { + v, ok := points[0].Value("error.type") + assert.True(t, ok) + assert.Equal(t, "panic", v.AsString()) + } +} + +func TestUnknownRequestBodySizeIsNotRecorded(t *testing.T) { + exporter, tp, reader, mp := newTestProviders() + + e := echo.New() + e.Use(NewMiddlewareWithConfig(Config{ServerName: "foobar", TracerProvider: tp, MeterProvider: mp})) + e.POST("/upload", func(c *echo.Context) error { + _, _ = io.Copy(io.Discard, c.Request().Body) + return c.NoContent(http.StatusNoContent) + }) + + r := httptest.NewRequest(http.MethodPost, "/upload", bytes.NewReader([]byte("chunked body"))) + r.ContentLength = -1 // unknown size, as for a chunked request + e.ServeHTTP(httptest.NewRecorder(), r) + + spans := exporter.GetSpans() + assert.Len(t, spans, 1) + for _, a := range spans[0].Attributes { + assert.NotEqual(t, attribute.Key("http.request.body.size"), a.Key) + } + + rm := metricdata.ResourceMetrics{} + assert.NoError(t, reader.Collect(t.Context(), &rm)) + for _, m := range rm.ScopeMetrics[0].Metrics { + if m.Name == "http.server.request.body.size" { + assert.Empty(t, m.Data.(metricdata.Histogram[int64]).DataPoints, "unknown size must not be recorded") + } + } +} + +func TestMetricsHaveNoClientChosenAttributes(t *testing.T) { + exporter, tp, reader, mp := newTestProviders() + + e := echo.New() + e.Use(NewMiddlewareWithConfig(Config{ServerName: "api.example.com", TracerProvider: tp, MeterProvider: mp})) + e.Any("/x", func(c *echo.Context) error { + return c.NoContent(http.StatusOK) + }) + + r := httptest.NewRequest("FOO", "/x", http.NoBody) + r.Host = "evil.example.com:1234" + e.ServeHTTP(httptest.NewRecorder(), r) + + spans := exporter.GetSpans() + assert.Len(t, spans, 1) + assert.Contains(t, spans[0].Attributes, attribute.String("server.address", "api.example.com")) + assert.Contains(t, spans[0].Attributes, attribute.String("http.request.method_original", "FOO")) + for _, a := range spans[0].Attributes { + assert.NotEqual(t, attribute.Key("server.port"), a.Key, "port must not come from the Host header when ServerName is set") + } + + points := durationPoints(t, reader) + if assert.Len(t, points, 1) { + assert.False(t, metricHas(points[0], "server.address")) + assert.False(t, metricHas(points[0], "server.port")) + assert.False(t, metricHas(points[0], "http.request.method_original")) + m, _ := points[0].Value("http.request.method") + assert.Equal(t, "_OTHER", m.AsString()) + } +} + +func TestHTTPRouteIsEchoRouteBehindServeMux(t *testing.T) { + testCases := []struct { + name string + pre bool + whenTarget string + expectName string + expectPath string + }{ + {name: "Pre middleware, matched route", pre: true, whenTarget: "/api/users/1", expectName: "GET /api/users/:id", expectPath: "/api/users/:id"}, + {name: "Use middleware, matched route", pre: false, whenTarget: "/api/users/1", expectName: "GET /api/users/:id", expectPath: "/api/users/:id"}, + {name: "Pre middleware, not found", pre: true, whenTarget: "/api/nothing", expectName: "GET"}, + {name: "Use middleware, not found", pre: false, whenTarget: "/api/nothing", expectName: "GET"}, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + exporter, tp, _, mp := newTestProviders() + + e := echo.New() + mw := NewMiddlewareWithConfig(Config{ServerName: "foobar", TracerProvider: tp, MeterProvider: mp}) + if tc.pre { + e.Pre(mw) + } else { + e.Use(mw) + } + e.GET("/api/users/:id", func(c *echo.Context) error { + return c.NoContent(http.StatusOK) + }) + + mux := http.NewServeMux() + mux.Handle("GET /api/", e) + + mux.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, tc.whenTarget, http.NoBody)) + + spans := exporter.GetSpans() + assert.Len(t, spans, 1) + assert.Equal(t, tc.expectName, spans[0].Name) + route := "" + for _, a := range spans[0].Attributes { + if a.Key == "http.route" { + route = a.Value.AsString() + } + } + assert.Equal(t, tc.expectPath, route) + }) + } +} + +func TestOnExtractionError(t *testing.T) { + var got error + e := echo.New() + e.Use(NewMiddlewareWithConfig(Config{OnExtractionError: func(c *echo.Context, err error) { got = err }})) + e.GET("/x", func(c *echo.Context) error { return c.NoContent(http.StatusOK) }) + + r := httptest.NewRequest(http.MethodGet, "/x", http.NoBody) + r.Host = "bad:host:name:1" + e.ServeHTTP(httptest.NewRecorder(), r) + + assert.Error(t, got) +} + +func TestSpanStartOptionsAndTracerKey(t *testing.T) { + exporter, tp, _, _ := newTestProviders() + + e := echo.New() + e.Use(NewMiddlewareWithConfig(Config{ + ServerName: "foobar", + TracerProvider: tp, + SpanStartOptions: []trace.SpanStartOption{trace.WithAttributes(attribute.String("custom", "yes"))}, + })) + e.GET("/x", func(c *echo.Context) error { + tracer, err := echo.ContextGet[trace.Tracer](c, TracerKey) + if assert.NoError(t, err) { + _, child := tracer.Start(c.Request().Context(), "child") + child.End() + } + return c.NoContent(http.StatusOK) + }) + e.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/x", http.NoBody)) + + spans := exporter.GetSpans() + assert.Len(t, spans, 2) + server := spans[1] + assert.Contains(t, server.Attributes, attribute.String("custom", "yes")) + assert.Equal(t, server.SpanContext.TraceID(), spans[0].SpanContext.TraceID(), "child span must be in the same trace") +} + +func TestSpanEndAttributesCallbackMustAppend(t *testing.T) { + testCases := []struct { + name string + callback AttributesFunc + expectErrType bool + }{ + { + name: "append to attr keeps error.type", + callback: func(c *echo.Context, v *Values, attr []attribute.KeyValue) []attribute.KeyValue { + return append(attr, attribute.String("extra", "1")) + }, + expectErrType: true, + }, + { + name: "new slice drops error.type", + callback: func(c *echo.Context, v *Values, attr []attribute.KeyValue) []attribute.KeyValue { + return []attribute.KeyValue{attribute.String("extra", "1")} + }, + expectErrType: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + exporter, tp, _, _ := newTestProviders() + e := echo.New() + e.Use(NewMiddlewareWithConfig(Config{ServerName: "foobar", TracerProvider: tp, SpanEndAttributes: tc.callback})) + e.GET("/x", func(c *echo.Context) error { return c.NoContent(http.StatusServiceUnavailable) }) + e.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/x", http.NoBody)) + + spans := exporter.GetSpans() + assert.Len(t, spans, 1) + assert.Contains(t, spans[0].Attributes, attribute.String("extra", "1")) + has := false + for _, a := range spans[0].Attributes { + if a.Key == "error.type" { + has = true + } + } + assert.Equal(t, tc.expectErrType, has) + }) + } +} + +func TestMultipartFormTempFilesAreRemoved(t *testing.T) { + var tmpFile string + e := echo.New() + e.Use(NewMiddleware("foobar")) + e.POST("/upload", func(c *echo.Context) error { + if err := c.Request().ParseMultipartForm(1); err != nil { // tiny memory limit forces a temp file + return err + } + f, err := c.Request().MultipartForm.File["file"][0].Open() + if err != nil { + return err + } + defer f.Close() + osFile, ok := f.(*os.File) + if assert.True(t, ok, "upload must be stored in a temp file") { + tmpFile = osFile.Name() + } + return c.NoContent(http.StatusOK) + }) + + body := &bytes.Buffer{} + mw := multipart.NewWriter(body) + fw, _ := mw.CreateFormFile("file", "big.txt") + _, _ = fw.Write(bytes.Repeat([]byte("x"), 64*1024)) + _ = mw.Close() + r := httptest.NewRequest(http.MethodPost, "/upload", body) + r.Header.Set("Content-Type", mw.FormDataContentType()) + w := httptest.NewRecorder() + e.ServeHTTP(w, r) + + assert.Equal(t, http.StatusOK, w.Result().StatusCode) + if assert.NotEmpty(t, tmpFile) { + _, err := os.Stat(tmpFile) + assert.True(t, os.IsNotExist(err), "temp file must be removed after the request") + } +} + +func TestPanicPaths(t *testing.T) { + testCases := []struct { + name string + recover bool + config func(cfg *Config) + handler echo.HandlerFunc + expectRepanic any + expectStatus int // 0 = no http.response.status_code attribute + expectErrorType string + }{ + { + name: "panic after 4xx response was sent is still an error", + recover: true, + handler: func(c *echo.Context) error { _ = c.NoContent(http.StatusNotFound); panic("late") }, + expectStatus: http.StatusNotFound, + expectErrorType: "panic", + }, + { + name: "panic with a status error records its status", + recover: true, + handler: func(c *echo.Context) error { panic(echo.ErrUnauthorized) }, + expectStatus: http.StatusUnauthorized, + expectErrorType: "panic", + }, + { + name: "http.ErrAbortHandler without response records no status", + recover: false, + handler: func(c *echo.Context) error { panic(http.ErrAbortHandler) }, + expectRepanic: http.ErrAbortHandler, + expectStatus: 0, + expectErrorType: "panic", + }, + { + name: "panic in OnNextError is recorded", + recover: true, + config: func(cfg *Config) { + cfg.OnNextError = func(c *echo.Context, err error) { panic("hook") } + }, + handler: func(c *echo.Context) error { return errors.New("x") }, + expectStatus: http.StatusInternalServerError, + expectErrorType: "panic", + }, + { + name: "panic in a recording callback keeps the original panic value", + recover: false, + config: func(cfg *Config) { + cfg.SpanEndAttributes = func(c *echo.Context, v *Values, attr []attribute.KeyValue) []attribute.KeyValue { + panic("callback") + } + }, + handler: func(c *echo.Context) error { panic("original") }, + expectRepanic: "original", + expectStatus: -1, // not checked, recording was interrupted + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + exporter, tp, _, mp := newTestProviders() + cfg := Config{ServerName: "foobar", TracerProvider: tp, MeterProvider: mp} + if tc.config != nil { + tc.config(&cfg) + } + e := echo.New() + if tc.recover { + e.Use(middleware.Recover()) + } + e.Use(NewMiddlewareWithConfig(cfg)) + e.GET("/x", tc.handler) + + var repanic any + func() { + defer func() { repanic = recover() }() + e.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/x", http.NoBody)) + }() + if tc.expectRepanic != nil { + assert.Equal(t, tc.expectRepanic, repanic) + } + if tc.expectStatus < 0 { + return + } + spans := exporter.GetSpans() + if !assert.Len(t, spans, 1) { + return + } + assert.Equal(t, codes.Error, spans[0].Status.Code) + var gotStatus int64 + var gotErrType string + for _, a := range spans[0].Attributes { + switch a.Key { + case "http.response.status_code": + gotStatus = a.Value.AsInt64() + case "error.type": + gotErrType = a.Value.AsString() + } + } + assert.Equal(t, int64(tc.expectStatus), gotStatus) + assert.Equal(t, tc.expectErrorType, gotErrType) + }) + } +} + +func TestRecoverAddedAfterMiddlewareReportsPanic(t *testing.T) { + exporter, tp, _, mp := newTestProviders() + e := echo.New() + e.Use(NewMiddlewareWithConfig(Config{ServerName: "foobar", TracerProvider: tp, MeterProvider: mp})) + e.Use(middleware.Recover()) // Recover inside turns the panic into *middleware.PanicStackError + e.GET("/x", func(c *echo.Context) error { panic("boom") }) + e.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/x", http.NoBody)) + + spans := exporter.GetSpans() + if assert.Len(t, spans, 1) { + assert.Equal(t, codes.Error, spans[0].Status.Code) + assert.Equal(t, "panic: boom", spans[0].Status.Description) + assert.Contains(t, spans[0].Attributes, attribute.String("error.type", "panic")) + } +} + +func TestServerNameWithoutHostIsRejected(t *testing.T) { + _, err := Config{ServerName: ":8080"}.ToMiddleware() + assert.Error(t, err) +} + +func BenchmarkMiddleware(b *testing.B) { + benchmarks := []struct { + name string + tp trace.TracerProvider + mp *sdkmetric.MeterProvider + }{ + {name: "noop providers"}, + {name: "sdk providers", tp: sdktrace.NewTracerProvider(), mp: sdkmetric.NewMeterProvider(sdkmetric.WithReader(sdkmetric.NewManualReader()))}, + } + for _, bm := range benchmarks { + b.Run(bm.name, func(b *testing.B) { + cfg := Config{ServerName: "foobar", TracerProvider: tracenoop.NewTracerProvider(), MeterProvider: metricnoop.NewMeterProvider()} + if bm.tp != nil { + cfg.TracerProvider = bm.tp + cfg.MeterProvider = bm.mp + } + e := echo.New() + e.Use(NewMiddlewareWithConfig(cfg)) + e.GET("/users/:id", func(c *echo.Context) error { return c.String(http.StatusOK, "ok") }) + r := httptest.NewRequest(http.MethodGet, "/users/123?x=1", http.NoBody) + r.Header.Set("User-Agent", "bench") + + b.ReportAllocs() + for b.Loop() { + e.ServeHTTP(httptest.NewRecorder(), r) + } + }) + } +} diff --git a/otel_test.go b/otel_test.go index b4ccb73..b9a7bb0 100644 --- a/otel_test.go +++ b/otel_test.go @@ -4,6 +4,7 @@ package echootel import ( + "errors" "net/http" "net/http/httptest" "strings" @@ -16,7 +17,7 @@ import ( "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/propagation" - "go.opentelemetry.io/otel/semconv/v1.39.0/httpconv" + "go.opentelemetry.io/otel/semconv/v1.40.0/httpconv" "go.opentelemetry.io/otel/trace" "go.opentelemetry.io/otel/trace/noop" @@ -106,24 +107,32 @@ func TestPropagationWithCustomPropagators(t *testing.T) { } func TestSkipper(t *testing.T) { - r := httptest.NewRequest(http.MethodGet, "/ping", http.NoBody) - w := httptest.NewRecorder() + exporter, tp, reader, mp := newTestProviders() skipper := func(c *echo.Context) bool { return c.Request().RequestURI == "/ping" } router := echo.New() - router.Use(NewMiddlewareWithConfig(Config{ServerName: "foobar", Skipper: skipper})) + router.Use(NewMiddlewareWithConfig(Config{ServerName: "foobar", Skipper: skipper, TracerProvider: tp, MeterProvider: mp})) router.GET("/ping", func(c *echo.Context) error { span := trace.SpanFromContext(c.Request().Context()) assert.False(t, span.SpanContext().HasSpanID()) assert.False(t, span.SpanContext().HasTraceID()) return c.NoContent(http.StatusOK) }) + router.GET("/other", func(c *echo.Context) error { + return c.NoContent(http.StatusOK) + }) - router.ServeHTTP(w, r) + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/ping", http.NoBody)) assert.Equal(t, http.StatusOK, w.Result().StatusCode, "should call the 'ping' handler") + assert.Len(t, exporter.GetSpans(), 0, "skipped request must not create a span") + assert.Len(t, durationPoints(t, reader), 0, "skipped request must not record metrics") + + router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/other", http.NoBody)) + assert.Len(t, exporter.GetSpans(), 1, "not skipped request must create a span") } func TestMetrics(t *testing.T) { @@ -141,11 +150,23 @@ func TestMetrics(t *testing.T) { attribute.Int64("http.response.status_code", 200), attribute.String("network.protocol.name", "http"), attribute.String("network.protocol.version", "1.1"), - attribute.String("server.address", "foobar"), attribute.String("url.scheme", "http"), attribute.String("http.route", "/user/:id"), }, }, + { + name: "request ended with an error", + whenRequestTarget: "/fail", + expectAttr: []attribute.KeyValue{ + attribute.String("http.request.method", "GET"), + attribute.Int64("http.response.status_code", 500), + attribute.String("network.protocol.name", "http"), + attribute.String("network.protocol.version", "1.1"), + attribute.String("url.scheme", "http"), + attribute.String("http.route", "/fail"), + attribute.String("error.type", "500"), + }, + }, { name: "request target not exist", whenRequestTarget: "/abc/123", @@ -154,7 +175,6 @@ func TestMetrics(t *testing.T) { attribute.Int64("http.response.status_code", 404), attribute.String("network.protocol.name", "http"), attribute.String("network.protocol.version", "1.1"), - attribute.String("server.address", "foobar"), attribute.String("url.scheme", "http"), }, }, @@ -178,7 +198,6 @@ func TestMetrics(t *testing.T) { attribute.Int64("http.response.status_code", 200), attribute.String("network.protocol.name", "http"), attribute.String("network.protocol.version", "1.1"), - attribute.String("server.address", "foobar"), attribute.String("url.scheme", "http"), attribute.String("http.route", "/user/:id"), attribute.String("key1", "value1"), @@ -208,6 +227,9 @@ func TestMetrics(t *testing.T) { assert.Equal(t, "123", id) return c.String(http.StatusOK, id) }) + e.GET("/fail", func(c *echo.Context) error { + return echo.NewHTTPError(http.StatusInternalServerError, "failed") + }) r := httptest.NewRequest(http.MethodGet, tc.whenRequestTarget, http.NoBody) w := httptest.NewRecorder() @@ -439,7 +461,6 @@ func TestNewMiddlewareWithConfig_Metric(t *testing.T) { attribute.Int64("http.response.status_code", 200), attribute.String("network.protocol.name", "http"), attribute.String("network.protocol.version", "1.1"), - attribute.String("server.address", "foobar"), attribute.String("url.scheme", "http"), attribute.String("http.route", "/user/:id"), }...), @@ -451,20 +472,30 @@ func TestNewMiddlewareWithConfig_Metric(t *testing.T) { func TestSpanStatusOnHTTP500(t *testing.T) { tests := []struct { - name string - handler echo.HandlerFunc + name string + handler echo.HandlerFunc + expectErrorType string }{ { name: "handler writes 500 status code directly", handler: func(c *echo.Context) error { return c.String(http.StatusInternalServerError, "internal server error") }, + expectErrorType: "500", }, { name: "handler returns echo HTTP error with 500", handler: func(c *echo.Context) error { return echo.NewHTTPError(http.StatusInternalServerError, "internal server error") }, + expectErrorType: "500", + }, + { + name: "handler returns plain error", + handler: func(c *echo.Context) error { + return errors.New("something failed") + }, + expectErrorType: "*errors.errorString", }, } @@ -489,6 +520,7 @@ func TestSpanStatusOnHTTP500(t *testing.T) { spans := exporter.GetSpans() assert.Len(t, spans, 1) assert.Equal(t, codes.Error, spans[0].Status.Code) + assert.Contains(t, spans[0].Attributes, attribute.String("error.type", tt.expectErrorType)) }) } } @@ -553,6 +585,10 @@ func TestSpanStatusOnHTTP4xx(t *testing.T) { assert.Len(t, spans, 1) // per the semantic conventions, span status is left unset for 4xx responses on server spans assert.Equal(t, codes.Unset, spans[0].Status.Code) + // ... and error.type is not set, as the request did not end with an error + for _, attr := range spans[0].Attributes { + assert.NotEqual(t, attribute.Key("error.type"), attr.Key) + } }) } } @@ -582,8 +618,18 @@ func TestConfig_OnNextError(t *testing.T) { r := httptest.NewRequest("GET", "/ping", http.NoBody) w := httptest.NewRecorder() + onNextErrorCalled := 0 + var onNextError OnErrorFunc + if tt.givenOnNextError != nil { + onNextError = func(c *echo.Context, err error) { + onNextErrorCalled++ + assert.ErrorIs(t, err, assert.AnError) + tt.givenOnNextError(c, err) + } + } + router := echo.New() - router.Use(NewMiddlewareWithConfig(Config{ServerName: "foobar", OnNextError: tt.givenOnNextError})) + router.Use(NewMiddlewareWithConfig(Config{ServerName: "foobar", OnNextError: onNextError})) router.GET("/ping", func(_ *echo.Context) error { return assert.AnError @@ -599,6 +645,67 @@ func TestConfig_OnNextError(t *testing.T) { router.ServeHTTP(w, r) assert.Equal(t, http.StatusTeapot, w.Result().StatusCode, "should call the 'ping' handler") assert.Equal(t, tt.wantHandlerCalled, handlerCalled, "handler called times mismatch") + if tt.givenOnNextError != nil { + assert.Equal(t, 1, onNextErrorCalled, "OnNextError must be called once") + } }) } } + +func TestHTTPRouteWithPreMiddleware(t *testing.T) { + exporter := tracetest.NewInMemoryExporter() + tp := sdktrace.NewTracerProvider(sdktrace.WithSyncer(exporter)) + reader := sdkmetric.NewManualReader() + mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + + e := echo.New() + e.Pre(NewMiddlewareWithConfig(Config{ServerName: "foobar", TracerProvider: tp, MeterProvider: mp})) + e.GET("/users/:id", func(c *echo.Context) error { + return c.String(http.StatusOK, c.Param("id")) + }) + + r := httptest.NewRequest(http.MethodGet, "/users/123", http.NoBody) + e.ServeHTTP(httptest.NewRecorder(), r) + + spans := exporter.GetSpans() + assert.Len(t, spans, 1) + assert.Equal(t, "GET /users/:id", spans[0].Name) + assert.Contains(t, spans[0].Attributes, attribute.String("http.route", "/users/:id")) + + rm := metricdata.ResourceMetrics{} + assert.NoError(t, reader.Collect(t.Context(), &rm)) + dp := rm.ScopeMetrics[0].Metrics[0].Data.(metricdata.Histogram[float64]).DataPoints[0] + route, ok := dp.Attributes.Value("http.route") + assert.True(t, ok) + assert.Equal(t, "/users/:id", route.AsString()) +} + +func TestCustomHTTPErrorHandlerWithOnNextError(t *testing.T) { + exporter := tracetest.NewInMemoryExporter() + tp := sdktrace.NewTracerProvider(sdktrace.WithSyncer(exporter)) + + e := echo.New() + e.HTTPErrorHandler = func(c *echo.Context, err error) { + if resp, uErr := echo.UnwrapResponse(c.Response()); uErr == nil && resp.Committed { + return + } + _ = c.NoContent(http.StatusTeapot) // custom mapping, not known to echo.ResolveResponseStatus + } + e.Use(NewMiddlewareWithConfig(Config{ + ServerName: "foobar", + TracerProvider: tp, + OnNextError: func(c *echo.Context, err error) { c.Echo().HTTPErrorHandler(c, err) }, + })) + e.GET("/teapot", func(c *echo.Context) error { + return errors.New("i am a teapot") + }) + + w := httptest.NewRecorder() + e.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/teapot", http.NoBody)) + + assert.Equal(t, http.StatusTeapot, w.Result().StatusCode) + spans := exporter.GetSpans() + assert.Len(t, spans, 1) + assert.Contains(t, spans[0].Attributes, attribute.Int("http.response.status_code", http.StatusTeapot)) + assert.Equal(t, codes.Unset, spans[0].Status.Code) +} diff --git a/version.go b/version.go index ea4b407..265b876 100644 --- a/version.go +++ b/version.go @@ -4,4 +4,4 @@ package echootel // Version is the current release version of the echo instrumentation. -const Version = "0.0.3" +const Version = "5.0.0"