Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to add metric attributes using otelhttp.WithMetricAttributesFn for http server #6540

Open
TanishqPorwar opened this issue Dec 29, 2024 · 2 comments
Labels
area: instrumentation Related to an instrumentation package bug Something isn't working instrumentation: otelhttp

Comments

@TanishqPorwar
Copy link
Contributor

Description

Unable to add metric attributes using otelhttp.WithMetricAttributesFn for http server. The attributes defined in the function are not included in the produced metrics.

Environment

  • OS: macos
  • Architecture: arm64
  • Go Version: go1.23.4 darwin/arm64
  • otelhttp: v0.58.0

Steps To Reproduce

  1. Using this code ...
package main

import (
	"context"
	"github.com/go-logr/stdr"
	"github.com/gorilla/mux"
	"github.com/prometheus/client_golang/prometheus/promhttp"
	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
	"go.opentelemetry.io/otel"
	"go.opentelemetry.io/otel/attribute"
	"go.opentelemetry.io/otel/exporters/otlp/otlptrace"
	"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
	"go.opentelemetry.io/otel/exporters/prometheus"
	"go.opentelemetry.io/otel/propagation"
	"go.opentelemetry.io/otel/sdk/metric"
	"go.opentelemetry.io/otel/sdk/resource"
	"go.opentelemetry.io/otel/sdk/trace"
	semconv "go.opentelemetry.io/otel/semconv/v1.27.0"
	t2 "go.opentelemetry.io/otel/trace"
	"log"
	"net/http"
	"os"
)

func TraceProvider() (tp t2.TracerProvider, meterProvider *metric.MeterProvider) {
	appName := os.Getenv("SERVICE_NAME")

	ctx := context.Background()

	res, err := resource.New(ctx,
		resource.WithFromEnv(),
		resource.WithProcess(),
		resource.WithTelemetrySDK(),
		resource.WithHost(),
		resource.WithAttributes(
			semconv.ServiceNameKey.String(appName),
			attribute.String("environment", "TEST"),
		),
	)

	promReader, err := prometheus.New()

	meterProvider = metric.NewMeterProvider(
		metric.WithResource(res),
		metric.WithReader(promReader),
	)

	sampler := trace.ParentBased(
		// TODO: remove this line
		trace.AlwaysSample(),
		//trace.TraceIDRatioBased(traceConfig.TraceSamplerRatio),
	)
	exporter, err := otlptrace.New(
		context.Background(),
		otlptracegrpc.NewClient(),
	)
	if err != nil {
		log.Fatal(err)
	}
	tp = trace.NewTracerProvider(
		trace.WithSampler(sampler),
		trace.WithBatcher(exporter),
		trace.WithResource(res),
	)

	otel.SetMeterProvider(meterProvider)
	otel.SetTracerProvider(tp)
	// TODO: remove this line
	otel.SetLogger(stdr.New(log.New(os.Stdout, "", log.LstdFlags|log.Lshortfile)))
	otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{}))
	return
}

func main() {
	// initialize the trace/metric provider
	tp, mp := TraceProvider()

	// create a new router
	router := mux.NewRouter()

	// register the health handler
	router.HandleFunc("/health", func(w http.ResponseWriter, _ *http.Request) {
		w.Header().Set("Content-Type", "application/json")
		w.WriteHeader(http.StatusOK)
		_, _ = w.Write([]byte("OK"))
	}).Methods(http.MethodGet)

	// register the metrics handler
	router.HandleFunc("/metrics", func(w http.ResponseWriter, r *http.Request) {
		promhttp.Handler().ServeHTTP(w, r)
	}).Methods(http.MethodGet)

	h := otelhttp.NewHandler(
		router,
		"",
		otelhttp.WithTracerProvider(tp),
		otelhttp.WithMeterProvider(mp),
		otelhttp.WithMetricAttributesFn(
			func(r *http.Request) []attribute.KeyValue {
				return []attribute.KeyValue{
					attribute.String("http.method", r.Method),
					attribute.String("http.route", r.URL.Path),
				}
			},
		),
	)

	srv := &http.Server{
		Handler: h,
		Addr:    ":8081",
	}

	log.Println("Listening on http://localhost:8081")
	err := srv.ListenAndServe()
	if err != nil {
		log.Fatal(err)
	}

}
  1. Run ...
go run main.go
curl "localhost:8081/health"
curl "localhost:8081/metrics"
  1. Observe the metrics output.
    Actual Behavior:
    The produced metrics do not include the attributes defined in WithMetricAttributesFn
...
http_server_duration_milliseconds_bucket{http_method="GET",http_scheme="http",http_status_code="200",net_host_name="localhost",net_host_port="8081",net_protocol_name="http",net_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_version="0.58.0",le="5000"}

Expected behavior

expected the produced metrics to include the attribute added in the WithMetricAttributesFn method.
eg.

...
http_server_duration_milliseconds_bucket{http_method="GET",http_scheme="http",http_status_code="200",net_host_name="localhost",net_host_port="8081",net_protocol_name="http",net_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_version="0.58.0",le="5000",http.route="/health"}

Additonal Context

I came across #5876 , which added support for custom attributes but it seems to only handle outbound HTTP requests through Transport struct. It looks like this behavior has not been extended to server-side metrics via semconv.HTTPServer .

Proposal and Contribution Inquiry:

Would it make sense to extend this support for HTTP server metrics, possibly in semconv.HTTPServer or a related component?

I’d be happy to contribute a PR if the maintainers are open to this improvement. Please let me know if this is something worth pursuing!

@TanishqPorwar TanishqPorwar added area: instrumentation Related to an instrumentation package bug Something isn't working instrumentation: otelhttp labels Dec 29, 2024
@TanishqPorwar
Copy link
Contributor Author

Update:
I’ve implemented a fix and raised a PR #6542

steps to validate

  1. update go.mod to fetch the changes from my branch:
replace go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp => github.com/TanishqPorwar/opentelemetry-go-contrib/instrumentation/net/http/otelhttp v0.0.0-20241229202604-9be79d190d2e
  1. Run the application and observe the updated metrics output.
http_server_duration_milliseconds_count{http_method="GET",http_route="/metrics",http_scheme="http",http_status_code="200",net_host_name="localhost",net_host_port="8081",net_protocol_name="http",net_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_version="0.58.0"}

I realize I went ahead with the code change without waiting for feedback on the issue. Please let me know if this approach aligns with the project’s direction, or if there are any suggestions for improvement!

@zhangtt0808
Copy link

zhangtt0808 commented Dec 30, 2024

@TanishqPorwar
I encountered the same issue. The MetricAttributesFn in config struct is not used by h *middleware. Hope your PR can solve this
issue.

func (h *middleware) configure(c *config) {
	h.tracer = c.Tracer
	h.propagators = c.Propagators
	h.spanStartOptions = c.SpanStartOptions
	h.readEvent = c.ReadEvent
	h.writeEvent = c.WriteEvent
	h.filters = c.Filters
	h.spanNameFormatter = c.SpanNameFormatter
	h.publicEndpoint = c.PublicEndpoint
	h.publicEndpointFn = c.PublicEndpointFn
	h.server = c.ServerName
	h.semconv = semconv.NewHTTPServer(c.Meter)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: instrumentation Related to an instrumentation package bug Something isn't working instrumentation: otelhttp
Projects
None yet
Development

No branches or pull requests

2 participants