Skip to content

Commit

Permalink
Rename SampledFilter to TraceBasedFilter (#5862)
Browse files Browse the repository at this point in the history
The specification calls this filter "TraceBased":
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#tracebased

I don't have a huge preference between the two, but lean towards
TraceBasedFilter to match the specification, and to match our
environment variable configuration (and probably the future file
configuration).

This PR must be merged or closed prior to the v1.31 release, since we
can't change it after the release.

---------

Co-authored-by: Tyler Yahn <[email protected]>
  • Loading branch information
dashpole and MrAlias authored Oct 2, 2024
1 parent cabe956 commit be328ac
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 10 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Added

- Add `go.opentelemetry.io/otel/sdk/metric/exemplar` package which includes `Exemplar`, `Filter`, `SampledFilter`, `AlwaysOnFilter`, `HistogramReservoir`, `FixedSizeReservoir`, `Reservoir`, `Value` and `ValueType` types. These will be used for configuring the exemplar reservoir for the metrics sdk. (#5747)
- Add `go.opentelemetry.io/otel/sdk/metric/exemplar` package which includes `Exemplar`, `Filter`, `TraceBasedFilter`, `AlwaysOnFilter`, `HistogramReservoir`, `FixedSizeReservoir`, `Reservoir`, `Value` and `ValueType` types. These will be used for configuring the exemplar reservoir for the metrics sdk. (#5747, #5862)

### Changed

Expand Down
2 changes: 1 addition & 1 deletion sdk/metric/exemplar.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func reservoirFunc[N int64 | float64](agg Aggregation) func() aggregate.Filtered
case "trace_based":
fallthrough
default:
filter = exemplar.SampledFilter
filter = exemplar.TraceBasedFilter
}

// https://github.com/open-telemetry/opentelemetry-specification/blob/d4b241f451674e8f611bb589477680341006ad2b/specification/metrics/sdk.md#exemplar-defaults
Expand Down
4 changes: 2 additions & 2 deletions sdk/metric/exemplar/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ import (
// Reservoir in making a sampling decision.
type Filter func(context.Context) bool

// SampledFilter is a [Filter] that will only offer measurements
// TraceBasedFilter is a [Filter] that will only offer measurements
// if the passed context associated with the measurement contains a sampled
// [go.opentelemetry.io/otel/trace.SpanContext].
func SampledFilter(ctx context.Context) bool {
func TraceBasedFilter(ctx context.Context) bool {
return trace.SpanContextFromContext(ctx).IsSampled()
}

Expand Down
12 changes: 6 additions & 6 deletions sdk/metric/exemplar/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ import (
"go.opentelemetry.io/otel/trace"
)

func TestSampledFilter(t *testing.T) {
t.Run("Int64", testSampledFiltered[int64])
t.Run("Float64", testSampledFiltered[float64])
func TestTraceBasedFilter(t *testing.T) {
t.Run("Int64", testTraceBasedFilter[int64])
t.Run("Float64", testTraceBasedFilter[float64])
}

func testSampledFiltered[N int64 | float64](t *testing.T) {
func testTraceBasedFilter[N int64 | float64](t *testing.T) {
ctx := context.Background()

assert.False(t, SampledFilter(ctx), "non-sampled context should not be offered")
assert.True(t, SampledFilter(sample(ctx)), "sampled context should be offered")
assert.False(t, TraceBasedFilter(ctx), "non-sampled context should not be offered")
assert.True(t, TraceBasedFilter(sample(ctx)), "sampled context should be offered")
}

func sample(parent context.Context) context.Context {
Expand Down

0 comments on commit be328ac

Please sign in to comment.