Skip to content

Commit dc8e2dd

Browse files
Permanently enable 'telemetry.newPipelineTelemetry' feature gate (#12856)
Discussed offline in relation to #12812 Introduction of this gate had some unintended side effects, such as removing attributes from loggers. --------- Co-authored-by: Jade Guiton <[email protected]>
1 parent 414308f commit dc8e2dd

File tree

13 files changed

+76
-116
lines changed

13 files changed

+76
-116
lines changed

.chloggen/lock-attributes-gate.yaml

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: breaking
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: service
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Set the `telemetry.newPipelineTelemetry` feature gate to stable.
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [12856]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: |
19+
The off state of this feature gate introduced a regression, where the Collector's internal logs were missing component attributes. See issue #12870 for more details on this bug. Since there does not appear to be much benefit to disabling the gate, we are immediately moving it to stable in order to lock in the correct behavior.
20+
21+
This comes with a breaking change, where internal logs exported through OTLP will now use instrumentation scope attributes to identify the source component instead of log attributes. This does not affect the Collector's stderr output. See the changelog for v0.123.0 for a more detailed description of the gate's effects.
22+
23+
# Optional: The change log or logs in which this entry should be included.
24+
# e.g. '[user]' or '[user, api]'
25+
# Include 'user' if the change is relevant to end users.
26+
# Include 'api' if there is a change to a library API.
27+
# Default: '[user]'
28+
change_logs: []

internal/telemetry/telemetry.go

+2-8
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ import (
1616

1717
var NewPipelineTelemetryGate = featuregate.GlobalRegistry().MustRegister(
1818
"telemetry.newPipelineTelemetry",
19-
featuregate.StageAlpha,
19+
featuregate.StageStable,
2020
featuregate.WithRegisterFromVersion("v0.123.0"),
21-
featuregate.WithRegisterDescription("Instruments Collector pipelines and injects component-identifying attributes"),
21+
featuregate.WithRegisterToVersion("v0.127.0"),
2222
featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/rfcs/component-universal-telemetry.md"),
2323
)
2424

@@ -46,16 +46,10 @@ type TelemetrySettings struct {
4646
// The publicization of this API is tracked in https://github.com/open-telemetry/opentelemetry-collector/issues/12405
4747

4848
func WithoutAttributes(ts TelemetrySettings, fields ...string) TelemetrySettings {
49-
if !NewPipelineTelemetryGate.IsEnabled() {
50-
return ts
51-
}
5249
return WithAttributeSet(ts, componentattribute.RemoveAttributes(ts.extraAttributes, fields...))
5350
}
5451

5552
func WithAttributeSet(ts TelemetrySettings, attrs attribute.Set) TelemetrySettings {
56-
if !NewPipelineTelemetryGate.IsEnabled() {
57-
return ts
58-
}
5953
ts.extraAttributes = attrs
6054
ts.Logger = componentattribute.ZapLoggerWithAttributes(ts.Logger, ts.extraAttributes)
6155
ts.TracerProvider = componentattribute.TracerProviderWithAttributes(ts.TracerProvider, ts.extraAttributes)

processor/memorylimiterprocessor/factory_test.go

-11
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717

1818
"go.opentelemetry.io/collector/component/componenttest"
1919
"go.opentelemetry.io/collector/consumer/consumertest"
20-
"go.opentelemetry.io/collector/featuregate"
2120
"go.opentelemetry.io/collector/internal/memorylimiter"
2221
"go.opentelemetry.io/collector/internal/telemetry"
2322
"go.opentelemetry.io/collector/internal/telemetry/componentattribute"
@@ -34,17 +33,7 @@ func TestCreateDefaultConfig(t *testing.T) {
3433
assert.NoError(t, componenttest.CheckConfigStruct(cfg))
3534
}
3635

37-
func setGate(t *testing.T, gate *featuregate.Gate, value bool) {
38-
initialValue := gate.IsEnabled()
39-
require.NoError(t, featuregate.GlobalRegistry().Set(gate.ID(), value))
40-
t.Cleanup(func() {
41-
_ = featuregate.GlobalRegistry().Set(gate.ID(), initialValue)
42-
})
43-
}
44-
4536
func TestCreateProcessor(t *testing.T) {
46-
setGate(t, telemetry.NewPipelineTelemetryGate, true)
47-
4837
factory := NewFactory()
4938
require.NotNil(t, factory)
5039

processor/memorylimiterprocessor/go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ require (
1111
go.opentelemetry.io/collector/consumer/consumererror v0.124.0
1212
go.opentelemetry.io/collector/consumer/consumertest v0.124.0
1313
go.opentelemetry.io/collector/consumer/xconsumer v0.124.0
14-
go.opentelemetry.io/collector/featuregate v1.30.0
1514
go.opentelemetry.io/collector/internal/memorylimiter v0.124.0
1615
go.opentelemetry.io/collector/internal/telemetry v0.124.0
1716
go.opentelemetry.io/collector/pdata v1.30.0
@@ -58,6 +57,7 @@ require (
5857
github.com/yusufpapurcu/wmi v1.2.4 // indirect
5958
go.opentelemetry.io/auto/sdk v1.1.0 // indirect
6059
go.opentelemetry.io/collector/component/componentstatus v0.124.0 // indirect
60+
go.opentelemetry.io/collector/featuregate v1.30.0 // indirect
6161
go.opentelemetry.io/collector/pdata/testdata v0.124.0 // indirect
6262
go.opentelemetry.io/contrib/bridges/otelzap v0.10.0 // indirect
6363
go.opentelemetry.io/otel/log v0.11.0 // indirect

receiver/otlpreceiver/factory_test.go

-11
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"go.opentelemetry.io/collector/consumer"
2222
"go.opentelemetry.io/collector/consumer/consumertest"
2323
"go.opentelemetry.io/collector/consumer/xconsumer"
24-
"go.opentelemetry.io/collector/featuregate"
2524
"go.opentelemetry.io/collector/internal/telemetry"
2625
"go.opentelemetry.io/collector/internal/telemetry/componentattribute"
2726
"go.opentelemetry.io/collector/internal/testutil"
@@ -37,17 +36,7 @@ func TestCreateDefaultConfig(t *testing.T) {
3736
assert.NoError(t, componenttest.CheckConfigStruct(cfg))
3837
}
3938

40-
func setGate(t *testing.T, gate *featuregate.Gate, value bool) {
41-
initialValue := gate.IsEnabled()
42-
require.NoError(t, featuregate.GlobalRegistry().Set(gate.ID(), value))
43-
t.Cleanup(func() {
44-
_ = featuregate.GlobalRegistry().Set(gate.ID(), initialValue)
45-
})
46-
}
47-
4839
func TestCreateSameReceiver(t *testing.T) {
49-
setGate(t, telemetry.NewPipelineTelemetryGate, true)
50-
5140
factory := NewFactory()
5241
cfg := factory.CreateDefaultConfig().(*Config)
5342
cfg.GRPC.NetAddr.Endpoint = testutil.GetAvailableLocalAddress(t)

receiver/otlpreceiver/go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ require (
2222
go.opentelemetry.io/collector/consumer/consumererror v0.124.0
2323
go.opentelemetry.io/collector/consumer/consumertest v0.124.0
2424
go.opentelemetry.io/collector/consumer/xconsumer v0.124.0
25-
go.opentelemetry.io/collector/featuregate v1.30.0
2625
go.opentelemetry.io/collector/internal/sharedcomponent v0.124.0
2726
go.opentelemetry.io/collector/internal/telemetry v0.124.0
2827
go.opentelemetry.io/collector/pdata v1.30.0
@@ -67,6 +66,7 @@ require (
6766
go.opentelemetry.io/collector/client v1.30.0 // indirect
6867
go.opentelemetry.io/collector/config/configcompression v1.30.0 // indirect
6968
go.opentelemetry.io/collector/extension/extensionauth v1.30.0 // indirect
69+
go.opentelemetry.io/collector/featuregate v1.30.0 // indirect
7070
go.opentelemetry.io/collector/pipeline v0.124.0 // indirect
7171
go.opentelemetry.io/contrib/bridges/otelzap v0.10.0 // indirect
7272
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.60.0 // indirect

service/go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ require (
4646
go.opentelemetry.io/collector/receiver/xreceiver v0.124.0
4747
go.opentelemetry.io/collector/semconv v0.124.0
4848
go.opentelemetry.io/collector/service/hostcapabilities v0.124.0
49-
go.opentelemetry.io/contrib/bridges/otelzap v0.10.0
5049
go.opentelemetry.io/contrib/otelconf v0.15.0
5150
go.opentelemetry.io/contrib/propagators/b3 v1.35.0
5251
go.opentelemetry.io/otel v1.35.0
@@ -108,6 +107,7 @@ require (
108107
go.opentelemetry.io/collector/config/configtls v1.30.0 // indirect
109108
go.opentelemetry.io/collector/consumer/consumererror v0.124.0 // indirect
110109
go.opentelemetry.io/collector/extension/extensionauth v1.30.0 // indirect
110+
go.opentelemetry.io/contrib/bridges/otelzap v0.10.0 // indirect
111111
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.60.0 // indirect
112112
go.opentelemetry.io/contrib/zpages v0.60.0 // indirect
113113
go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.11.0 // indirect

service/internal/graph/connector.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,12 @@ func (n *connectorNode) buildComponent(
4949
builder *builders.ConnectorBuilder,
5050
nexts []baseConsumer,
5151
) error {
52-
set := connector.Settings{ID: n.componentID, TelemetrySettings: tel, BuildInfo: info}
53-
if telemetry.NewPipelineTelemetryGate.IsEnabled() {
54-
set.TelemetrySettings = telemetry.WithAttributeSet(set.TelemetrySettings, *n.Set())
52+
set := connector.Settings{
53+
ID: n.componentID,
54+
TelemetrySettings: telemetry.WithAttributeSet(tel, *n.Set()),
55+
BuildInfo: info,
5556
}
57+
5658
switch n.rcvrPipelineType {
5759
case pipeline.SignalTraces:
5860
return n.buildTraces(ctx, set, builder, nexts)

service/internal/graph/exporter.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,12 @@ func (n *exporterNode) buildComponent(
4545
info component.BuildInfo,
4646
builder *builders.ExporterBuilder,
4747
) error {
48-
set := exporter.Settings{ID: n.componentID, TelemetrySettings: tel, BuildInfo: info}
49-
if telemetry.NewPipelineTelemetryGate.IsEnabled() {
50-
set.TelemetrySettings = telemetry.WithAttributeSet(set.TelemetrySettings, *n.Set())
48+
set := exporter.Settings{
49+
ID: n.componentID,
50+
TelemetrySettings: telemetry.WithAttributeSet(tel, *n.Set()),
51+
BuildInfo: info,
5152
}
53+
5254
var err error
5355
switch n.pipelineType {
5456
case pipeline.SignalTraces:

service/internal/graph/processor.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,12 @@ func (n *processorNode) buildComponent(ctx context.Context,
4747
builder *builders.ProcessorBuilder,
4848
next baseConsumer,
4949
) error {
50-
set := processor.Settings{ID: n.componentID, TelemetrySettings: tel, BuildInfo: info}
51-
if telemetry.NewPipelineTelemetryGate.IsEnabled() {
52-
set.TelemetrySettings = telemetry.WithAttributeSet(set.TelemetrySettings, *n.Set())
50+
set := processor.Settings{
51+
ID: n.componentID,
52+
TelemetrySettings: telemetry.WithAttributeSet(tel, *n.Set()),
53+
BuildInfo: info,
5354
}
55+
5456
var err error
5557
switch n.pipelineID.Signal() {
5658
case pipeline.SignalTraces:

service/internal/graph/receiver.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,12 @@ func (n *receiverNode) buildComponent(ctx context.Context,
4242
builder *builders.ReceiverBuilder,
4343
nexts []baseConsumer,
4444
) error {
45-
set := receiver.Settings{ID: n.componentID, TelemetrySettings: tel, BuildInfo: info}
46-
if telemetry.NewPipelineTelemetryGate.IsEnabled() {
47-
set.TelemetrySettings = telemetry.WithAttributeSet(set.TelemetrySettings, *n.Set())
45+
set := receiver.Settings{
46+
ID: n.componentID,
47+
TelemetrySettings: telemetry.WithAttributeSet(tel, *n.Set()),
48+
BuildInfo: info,
4849
}
50+
4951
var err error
5052
switch n.pipelineType {
5153
case pipeline.SignalTraces:

service/telemetry/logger.go

+14-41
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,11 @@
44
package telemetry // import "go.opentelemetry.io/collector/service/telemetry"
55

66
import (
7-
"go.opentelemetry.io/contrib/bridges/otelzap"
87
"go.opentelemetry.io/otel/attribute"
98
"go.opentelemetry.io/otel/log"
109
"go.uber.org/zap"
1110
"go.uber.org/zap/zapcore"
1211

13-
"go.opentelemetry.io/collector/internal/telemetry"
1412
"go.opentelemetry.io/collector/internal/telemetry/componentattribute"
1513
)
1614

@@ -43,54 +41,29 @@ func newLogger(set Settings, cfg Config) (*zap.Logger, log.LoggerProvider, error
4341

4442
var lp log.LoggerProvider
4543

46-
if telemetry.NewPipelineTelemetryGate.IsEnabled() {
47-
logger = logger.WithOptions(zap.WrapCore(func(core zapcore.Core) zapcore.Core {
48-
core = componentattribute.NewConsoleCoreWithAttributes(core, attribute.NewSet())
44+
logger = logger.WithOptions(zap.WrapCore(func(core zapcore.Core) zapcore.Core {
45+
core = componentattribute.NewConsoleCoreWithAttributes(core, attribute.NewSet())
4946

50-
if len(cfg.Logs.Processors) > 0 && set.SDK != nil {
51-
lp = set.SDK.LoggerProvider()
52-
53-
core = componentattribute.NewOTelTeeCoreWithAttributes(
54-
core,
55-
lp,
56-
"go.opentelemetry.io/collector/service/telemetry",
57-
cfg.Logs.Level,
58-
attribute.NewSet(),
59-
)
60-
}
61-
62-
if cfg.Logs.Sampling != nil && cfg.Logs.Sampling.Enabled {
63-
core = componentattribute.NewWrapperCoreWithAttributes(core, func(c zapcore.Core) zapcore.Core {
64-
return newSampledCore(c, cfg.Logs.Sampling)
65-
})
66-
}
67-
68-
return core
69-
}))
70-
} else {
7147
if len(cfg.Logs.Processors) > 0 && set.SDK != nil {
7248
lp = set.SDK.LoggerProvider()
7349

74-
logger = logger.WithOptions(zap.WrapCore(func(c zapcore.Core) zapcore.Core {
75-
core, err := zapcore.NewIncreaseLevelCore(zapcore.NewTee(
76-
c,
77-
otelzap.NewCore("go.opentelemetry.io/collector/service/telemetry",
78-
otelzap.WithLoggerProvider(lp),
79-
),
80-
), zap.NewAtomicLevelAt(cfg.Logs.Level))
81-
if err != nil {
82-
panic(err)
83-
}
84-
return core
85-
}))
50+
core = componentattribute.NewOTelTeeCoreWithAttributes(
51+
core,
52+
lp,
53+
"go.opentelemetry.io/collector/service/telemetry",
54+
cfg.Logs.Level,
55+
attribute.NewSet(),
56+
)
8657
}
8758

8859
if cfg.Logs.Sampling != nil && cfg.Logs.Sampling.Enabled {
89-
logger = logger.WithOptions(zap.WrapCore(func(c zapcore.Core) zapcore.Core {
60+
core = componentattribute.NewWrapperCoreWithAttributes(core, func(c zapcore.Core) zapcore.Core {
9061
return newSampledCore(c, cfg.Logs.Sampling)
91-
}))
62+
})
9263
}
93-
}
64+
65+
return core
66+
}))
9467

9568
return logger, lp, nil
9669
}

service/telemetry/logger_test.go

+9-30
Original file line numberDiff line numberDiff line change
@@ -13,26 +13,14 @@ import (
1313
"github.com/stretchr/testify/require"
1414
config "go.opentelemetry.io/contrib/otelconf/v0.3.0"
1515
"go.uber.org/zap/zapcore"
16-
17-
"go.opentelemetry.io/collector/featuregate"
18-
"go.opentelemetry.io/collector/internal/telemetry"
1916
)
2017

21-
func setGate(t *testing.T, gate *featuregate.Gate, value bool) {
22-
initialValue := gate.IsEnabled()
23-
require.NoError(t, featuregate.GlobalRegistry().Set(gate.ID(), value))
24-
t.Cleanup(func() {
25-
_ = featuregate.GlobalRegistry().Set(gate.ID(), initialValue)
26-
})
27-
}
28-
2918
func TestNewLogger(t *testing.T) {
3019
tests := []struct {
31-
name string
32-
wantCoreType any
33-
wantCoreTypeRfc any
34-
wantErr error
35-
cfg Config
20+
name string
21+
wantCoreType any
22+
wantErr error
23+
cfg Config
3624
}{
3725
{
3826
name: "no log config",
@@ -52,8 +40,7 @@ func TestNewLogger(t *testing.T) {
5240
InitialFields: map[string]any{"fieldKey": "filed-value"},
5341
},
5442
},
55-
wantCoreType: "*zapcore.ioCore",
56-
wantCoreTypeRfc: "*componentattribute.consoleCoreWithAttributes",
43+
wantCoreType: "*componentattribute.consoleCoreWithAttributes",
5744
},
5845
{
5946
name: "log config with processors",
@@ -76,8 +63,7 @@ func TestNewLogger(t *testing.T) {
7663
},
7764
},
7865
},
79-
wantCoreType: "*zapcore.levelFilterCore",
80-
wantCoreTypeRfc: "*componentattribute.otelTeeCoreWithAttributes",
66+
wantCoreType: "*componentattribute.otelTeeCoreWithAttributes",
8167
},
8268
{
8369
name: "log config with sampling",
@@ -99,8 +85,7 @@ func TestNewLogger(t *testing.T) {
9985
InitialFields: map[string]any(nil),
10086
},
10187
},
102-
wantCoreType: "*zapcore.sampler",
103-
wantCoreTypeRfc: "*componentattribute.wrapperCoreWithAttributes",
88+
wantCoreType: "*componentattribute.wrapperCoreWithAttributes",
10489
},
10590
}
10691
for _, tt := range tests {
@@ -125,13 +110,7 @@ func TestNewLogger(t *testing.T) {
125110
}
126111
}
127112
}
128-
t.Run(tt.name, func(t *testing.T) {
129-
setGate(t, telemetry.NewPipelineTelemetryGate, false)
130-
testCoreType(t, tt.wantCoreType)
131-
})
132-
t.Run(tt.name+" (pipeline telemetry on)", func(t *testing.T) {
133-
setGate(t, telemetry.NewPipelineTelemetryGate, true)
134-
testCoreType(t, tt.wantCoreTypeRfc)
135-
})
113+
114+
testCoreType(t, tt.wantCoreType)
136115
}
137116
}

0 commit comments

Comments
 (0)