Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Sovietaced committed Dec 28, 2023
1 parent 5886e69 commit 0c09720
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 51 deletions.
13 changes: 6 additions & 7 deletions instrumentation/net/http/otelhttp/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,16 @@ const (

// Server HTTP metrics.
const (
ServerRequestCount = "http.server.request_count" // Incoming request count total
ServerRequestContentLength = "http.server.request_content_length" // Incoming request bytes total
ServerResponseContentLength = "http.server.response_content_length" // Incoming response bytes total
ServerLatency = "http.server.duration" // Incoming end to end duration, milliseconds
ServerRequestSize = "http.server.request.size" // Incoming request bytes total
ServerResponseSize = "http.server.response.size" // Incoming response bytes total
ServerDuration = "http.server.duration" // Incoming end to end duration, milliseconds
)

// Client HTTP metrics.
const (
ClientRequestContentLength = "http.client.request_content_length" // Outgoing request bytes total
ClientResponseContentLength = "http.client.response_content_length" // Outgoing response bytes total
ClientLatency = "http.client.duration" // Outgoing end to end duration, milliseconds
ClientRequestSize = "http.client.request.size" // Outgoing request bytes total
ClientResponseSize = "http.client.response.size" // Outgoing response bytes total
ClientDuration = "http.client.duration" // Outgoing end to end duration, milliseconds
)

// Filter is a predicate used to determine whether a given http.request should
Expand Down
6 changes: 3 additions & 3 deletions instrumentation/net/http/otelhttp/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,21 +108,21 @@ func handleErr(err error) {
func (h *middleware) createMeasures() {
var err error
h.requestBytesCounter, err = h.meter.Int64Counter(
ServerRequestContentLength,
ServerRequestSize,
metric.WithUnit("By"),
metric.WithDescription("Measures the size of HTTP request content length (uncompressed)"),
)
handleErr(err)

h.responseBytesCounter, err = h.meter.Int64Counter(
ServerResponseContentLength,
ServerResponseSize,
metric.WithUnit("By"),
metric.WithDescription("Measures the size of HTTP response content length (uncompressed)"),
)
handleErr(err)

h.serverLatencyMeasure, err = h.meter.Float64Histogram(
ServerLatency,
ServerDuration,
metric.WithUnit("ms"),
metric.WithDescription("Measures the duration of HTTP request handling"),
)
Expand Down
25 changes: 12 additions & 13 deletions instrumentation/net/http/otelhttp/test/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func assertScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribut
require.Len(t, sm.Metrics, 3)

want := metricdata.Metrics{
Name: "http.server.request_content_length",
Name: "http.server.request.size",
Description: "Measures the size of HTTP request content length (uncompressed)",
Unit: "By",
Data: metricdata.Sum[int64]{
Expand All @@ -62,7 +62,7 @@ func assertScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribut
metricdatatest.AssertEqual(t, want, sm.Metrics[0], metricdatatest.IgnoreTimestamp())

want = metricdata.Metrics{
Name: "http.server.response_content_length",
Name: "http.server.response.size",
Description: "Measures the size of HTTP response content length (uncompressed)",
Unit: "By",
Data: metricdata.Sum[int64]{
Expand All @@ -73,17 +73,16 @@ func assertScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribut
}
metricdatatest.AssertEqual(t, want, sm.Metrics[1], metricdatatest.IgnoreTimestamp())

// Duration value is not predictable.
dur := sm.Metrics[2]
assert.Equal(t, "http.server.duration", dur.Name)
require.IsType(t, dur.Data, metricdata.Histogram[float64]{})
hist := dur.Data.(metricdata.Histogram[float64])
assert.Equal(t, metricdata.CumulativeTemporality, hist.Temporality)
require.Len(t, hist.DataPoints, 1)
dPt := hist.DataPoints[0]
assert.Equal(t, attrs, dPt.Attributes, "attributes")
assert.Equal(t, uint64(1), dPt.Count, "count")
assert.Equal(t, []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}, dPt.Bounds, "bounds")
want = metricdata.Metrics{
Name: "http.server.duration",
Description: "Measures the duration of HTTP request handling",
Unit: "ms",
Data: metricdata.Histogram[float64]{
DataPoints: []metricdata.HistogramDataPoint[float64]{{Attributes: attrs}},
Temporality: metricdata.CumulativeTemporality,
},
}
metricdatatest.AssertEqual(t, want, sm.Metrics[2], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue())
}

func TestHandlerBasics(t *testing.T) {
Expand Down
35 changes: 13 additions & 22 deletions instrumentation/net/http/otelhttp/test/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,46 +319,37 @@ func assertClientScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs at
require.Len(t, sm.Metrics, 3)

want := metricdata.Metrics{
Name: "http.client.request_content_length",
Name: "http.client.request.size",
Data: metricdata.Sum[int64]{
DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 4}},
Temporality: metricdata.CumulativeTemporality,
IsMonotonic: true,
},
Description: "Measures the size of HTTP request content length (uncompressed)",
Description: "Measures the size of HTTP request messages.",
Unit: "By",
}
metricdatatest.AssertEqual(t, want, sm.Metrics[0], metricdatatest.IgnoreTimestamp())

want = metricdata.Metrics{
Name: "http.client.response_content_length",
Name: "http.client.response.size",
Data: metricdata.Sum[int64]{
DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 13}},
Temporality: metricdata.CumulativeTemporality,
IsMonotonic: true,
},
Description: "Measures the size of HTTP response content length (uncompressed)",
Description: "Measures the size of HTTP response messages.",
Unit: "By",
}
metricdatatest.AssertEqual(t, want, sm.Metrics[1], metricdatatest.IgnoreTimestamp())

dur := sm.Metrics[2]
assert.Equal(t, "http.client.duration", dur.Name)
require.IsType(t, dur.Data, metricdata.Histogram[float64]{})
hist := dur.Data.(metricdata.Histogram[float64])
assert.Equal(t, metricdata.CumulativeTemporality, hist.Temporality)
require.Len(t, hist.DataPoints, 1)
dPt := hist.DataPoints[0]
assert.Equal(t, attrs, dPt.Attributes, "attributes")
assert.Equal(t, uint64(1), dPt.Count, "count")
assert.Equal(t, []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}, dPt.Bounds, "bounds")

// Duration value is not deterministic because the code does not support a pluggable clock.
// So just value that one of the buckets has been incremented.
bucketSum := uint64(0)
for _, bucketCount := range dPt.BucketCounts {
bucketSum += bucketCount
want = metricdata.Metrics{
Name: "http.client.duration",
Data: metricdata.Histogram[float64]{
DataPoints: []metricdata.HistogramDataPoint[float64]{{Attributes: attrs}},
Temporality: metricdata.CumulativeTemporality,
},
Description: "Measures the duration of outbound HTTP requests.",
Unit: "ms",
}

require.Equal(t, uint64(1), bucketSum)
metricdatatest.AssertEqual(t, want, sm.Metrics[2], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue())
}
12 changes: 6 additions & 6 deletions instrumentation/net/http/otelhttp/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,23 +93,23 @@ func (t *Transport) applyConfig(c *config) {
func (t *Transport) createMeasures() {
var err error
t.requestBytesCounter, err = t.meter.Int64Counter(
ClientRequestContentLength,
ClientRequestSize,
metric.WithUnit("By"),
metric.WithDescription("Measures the size of HTTP request content length (uncompressed)"),
metric.WithDescription("Measures the size of HTTP request messages."),
)
handleErr(err)

t.responseBytesCounter, err = t.meter.Int64Counter(
ClientResponseContentLength,
ClientResponseSize,
metric.WithUnit("By"),
metric.WithDescription("Measures the size of HTTP response content length (uncompressed)"),
metric.WithDescription("Measures the size of HTTP response messages."),
)
handleErr(err)

t.latencyMeasure, err = t.meter.Float64Histogram(
ClientLatency,
ClientDuration,
metric.WithUnit("ms"),
metric.WithDescription("Measures the duration of HTTP request handling"),
metric.WithDescription("Measures the duration of outbound HTTP requests."),
)
handleErr(err)
}
Expand Down

0 comments on commit 0c09720

Please sign in to comment.