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

Add metrics for otelhttp client #427

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions instrumentation/net/http/otelhttp/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ const (
ServerLatency = "http.server.duration" // Incoming end to end duration, microseconds
)

// Client HTTP metric instrument names
vchitai marked this conversation as resolved.
Show resolved Hide resolved
const (
// clientRequestDuration is the name of the instrument that measures the duration of outbound HTTP requests.
clientRequestDuration = "http.client.duration"
)

// Filter is a predicate used to determine whether a given http.request should
// be traced. A Filter must return true if the request should be traced.
type Filter func(*http.Request) bool
143 changes: 143 additions & 0 deletions instrumentation/net/http/otelhttp/metrics.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package otelhttp

import (
"context"
"io"
"net/http"
"sync"
"time"

"go.opentelemetry.io/otel/semconv"

"go.opentelemetry.io/otel/unit"

"go.opentelemetry.io/otel/api/metric"
"go.opentelemetry.io/otel/label"
)

type metricsTransport struct {
vchitai marked this conversation as resolved.
Show resolved Hide resolved
meter metric.Meter
base *Transport
clientDurationRecorder metric.Float64ValueRecorder
}

type tracker struct {
ctx context.Context
start time.Time
body io.ReadCloser
endOnce sync.Once
labels []label.KeyValue

// recorders
vchitai marked this conversation as resolved.
Show resolved Hide resolved
clientDurationRecorder metric.Float64ValueRecorder
}

func (trans *metricsTransport) applyConfig(c *config) {
trans.base.applyConfig(c)

trans.meter = c.Meter
trans.createMeasures()
}

// RoundTrip implements http.RoundTripper, delegating to Base and recording stats for the request.
func (trans *metricsTransport) RoundTrip(req *http.Request) (*http.Response, error) {
labels := semconv.HTTPClientAttributesFromHTTPRequest(req)

ctx := req.Context()
tracker := &tracker{
start: time.Now(),
ctx: ctx,
clientDurationRecorder: trans.clientDurationRecorder,
}

// Perform request.;
vchitai marked this conversation as resolved.
Show resolved Hide resolved
resp, err := trans.base.RoundTrip(req)
if err != nil {
tracker.labels = append(labels, semconv.HTTPAttributesFromHTTPStatusCode(http.StatusInternalServerError)...)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should include the response status not http.StatusInternalServerError

Copy link
Author

Choose a reason for hiding this comment

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

In case of HTTP connection error occurred, should we recorded nothing or 500 as the HTTP status code. What's your opinion?

tracker.end()
} else {
tracker.labels = append(labels, semconv.HTTPAttributesFromHTTPStatusCode(http.StatusInternalServerError)...)
vchitai marked this conversation as resolved.
Show resolved Hide resolved
if resp.Body == nil {
tracker.end()
} else {
tracker.body = resp.Body
resp.Body = wrappedBodyIO(tracker, resp.Body)
}
}
return resp, err
}

// wrappedBodyIO returns a wrapped version of the original
// Body and only implements the same combination of additional
// interfaces as the original.
func wrappedBodyIO(wrapper io.ReadCloser, body io.ReadCloser) io.ReadCloser {
wr, i0 := body.(io.Writer)
switch {
case !i0:
return struct {
io.ReadCloser
}{wrapper}

case i0:
return struct {
io.ReadCloser
io.Writer
}{wrapper, wr}
default:
return struct {
io.ReadCloser
}{wrapper}
}
vchitai marked this conversation as resolved.
Show resolved Hide resolved
}

func (trans *metricsTransport) createMeasures() {
var err error
trans.clientDurationRecorder, err = trans.meter.NewFloat64ValueRecorder(
clientRequestDuration,
metric.WithDescription("measure the duration of the outbound HTTP request"),
vchitai marked this conversation as resolved.
Show resolved Hide resolved
metric.WithUnit(unit.Milliseconds),
)
handleErr(err)
}

var _ io.ReadCloser = (*tracker)(nil)

func (tracker *tracker) end() {
tracker.endOnce.Do(func() {
latencyMs := float64(time.Since(tracker.start)) / float64(time.Millisecond)
tracker.clientDurationRecorder.Record(tracker.ctx, latencyMs, tracker.labels...)
})
}

func (tracker *tracker) Read(b []byte) (int, error) {
n, err := tracker.body.Read(b)
switch err {
case nil:
return n, nil
case io.EOF:
tracker.end()
}
return n, err
}

func (tracker *tracker) Close() error {
// Invoking endSpan on Close will help catch the cases
// in which a read returned a non-nil error, we set the
// span status but didn't end the span.
vchitai marked this conversation as resolved.
Show resolved Hide resolved
tracker.end()
return tracker.body.Close()
}
8 changes: 5 additions & 3 deletions instrumentation/net/http/otelhttp/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@ var _ http.RoundTripper = &Transport{}

// NewTransport wraps the provided http.RoundTripper with one that
// starts a span and injects the span context into the outbound request headers.
func NewTransport(base http.RoundTripper, opts ...Option) *Transport {
t := Transport{
rt: base,
func NewTransport(base http.RoundTripper, opts ...Option) http.RoundTripper {
Copy link
Member

Choose a reason for hiding this comment

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

Since this is the primary mechanism by which I expect otelhttp.Transport instances to be created and this leaves no configuration option for enabling/disabling metric collection, does it make more sense to integrate the metrics collection directly in the Transport.RoundTrip method? This would also be more in line with the approach taken by the otelhttp.Handler.

Copy link
Author

Choose a reason for hiding this comment

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

I prefer to split up the implementation of metrics and tracing parts for ease of maintenance.

t := metricsTransport{
base: &Transport{
rt: base,
},
}

defaultOpts := []Option{
Expand Down