Skip to content

Commit 3591009

Browse files
StacieClark-Elasticmergify[bot]
StacieClark-Elastic
authored and
mergify[bot]
committed
Fix/race condition cel metrics (#48321)
* fix/race-condition-cel-metrics Fixed a race condition in exporter_factory.go GetExporter that could cause too many exporter connections to be created. New test to catch race condition. (cherry picked from commit 990735b)
1 parent 4c3cfd3 commit 3591009

4 files changed

Lines changed: 121 additions & 52 deletions

File tree

‎changelog/fragments/1764199697-fix-race-condition-exporter-factory.yaml‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# REQUIRED
2+
# Kind can be one of:
3+
# - breaking-change: a change to previously-documented behavior
4+
# - deprecation: functionality that is being removed in a later release
5+
# - bug-fix: fixes a problem in a previous version
6+
# - enhancement: extends functionality but does not break or fix existing behavior
7+
# - feature: new functionality
8+
# - known-issue: problems that we are aware of in a given version
9+
# - security: impacts on the security of a product or a user’s deployment.
10+
# - upgrade: important information for someone upgrading from a prior version
11+
# - other: does not fit into any of the other categories
12+
kind: bug-fix
13+
14+
# REQUIRED for all kinds
15+
# Change summary; a 80ish characters long description of the change.
16+
summary:
17+
Fixed an issue in the initialization of exporters for input metrics that could cause an unexpected number of exporter
18+
connections to be created.
19+
20+
# REQUIRED for breaking-change, deprecation, known-issue
21+
# Long description; in case the summary is not enough to describe the change
22+
# this field accommodate a description without length limits.
23+
# description:
24+
25+
# REQUIRED for breaking-change, deprecation, known-issue
26+
# impact:
27+
28+
# REQUIRED for breaking-change, deprecation, known-issue
29+
# action:
30+
31+
# REQUIRED for all kinds
32+
# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
33+
component: filebeat
34+
35+
# AUTOMATED
36+
# OPTIONAL to manually add other PR URLs
37+
# PR URL: A link the PR that added the changeset.
38+
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
39+
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
40+
# Please provide it if you are adding a fragment for a different PR.
41+
# pr: https://github.com/owner/repo/1234
42+
43+
# AUTOMATED
44+
# OPTIONAL to manually add other issue URLs
45+
# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
46+
# If not present is automatically filled by the tooling with the issue linked to the PR number.
47+
# issue: https://github.com/owner/repo/1234

‎x-pack/filebeat/input/cel/doc.go‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -57,24 +57,25 @@ https://github.com/elastic/beats/tree/main/x-pack/filebeat/input/cel/cel_metric_
5757
5858
CEL Metrics exported for each periodic run:
5959
60-
Name Description Metric Type
61-
62-
input.cel.periodic.run the number of times a periodic run was started. Int64Counter
63-
input.cel.periodic.program.run.started the number of times a program was started in a periodic run. Int64counter
64-
input.cel.periodic.program.run.success the number of times a program terminated without an error in a periodic run. Int64counter
65-
input.cel.periodic.batch.received the number of the number of batches generated in a periodic run. Int64counter
66-
input.cel.periodic.batch.published the number of the number of batches successfully published in a periodic run. Int64counter
67-
input.cel.periodic.event.received the number of the number of events generated in a periodic run. Int64counter
68-
input.cel.periodic.event.published the number of the number of events published in a periodic run. Int64counter
69-
input.cel.periodic.run.duration the total duration of time in seconds spent in a periodic run. Float64Counter
70-
input.cel.periodic.cel.duration the total duration of time in seconds spent processing CEL programs in a periodic run. Float64Histogram
71-
input.cel.periodic.event.publish.duration the total duration of time in seconds publishing events in a periodic run. Float64Histogram
72-
input.cel.program.batch.received the number of batches the program has generated. Int64Histogram
73-
input.cel.program.event.published the number of events the program has published. Int64Histogram
74-
input.cel.program.batch.published the number of batched the program has published. Int64Histogram
75-
input.cel.program.run.duration the total time in seconds spent executing the program. Float64Histogram
76-
input.cel.program.cel.duration the total time in seconds spent processing the CEL program. Float64Histogram
77-
input.cel.program.publish.duration the total time in seconds spent publishing in the program. Float64Histogram
60+
Name Description Metric Type
61+
62+
input.cel.periodic.run the number of times a periodic run was started. Int64Counter
63+
input.cel.periodic.program.run.started the number of times a program was started in a periodic run. Int64Counter
64+
input.cel.periodic.program.run.success the number of times a program terminated without an error in a periodic run. Int64Counter
65+
input.cel.periodic.batch.received the number of the number of batches generated in a periodic run. Int64Counter
66+
input.cel.periodic.batch.published the number of the number of batches successfully published in a periodic run. Int64Counter
67+
input.cel.periodic.event.received the number of the number of events generated in a periodic run. Int64Counter
68+
input.cel.periodic.event.published the number of the number of events published in a periodic run. Int64Counter
69+
input.cel.periodic.run.duration the total duration of time in seconds spent in a periodic run. Float64Counter
70+
input.cel.periodic.cel.duration the total duration of time in seconds spent processing CEL programs in a periodic run. Float64Histogram
71+
input.cel.periodic.event.publish.duration the total duration of time in seconds publishing events in a periodic run. Float64Histogram
72+
input.cel.program.batch.received the number of batches the program has generated. Int64Histogram
73+
input.cel.program.event.received the number of events the program has generated. Int64Histogram
74+
input.cel.program.batch.published the number of batches the program has published. Int64Histogram
75+
input.cel.program.event.published the number of events the program has published. Int64Histogram
76+
input.cel.program.run.duration the time in seconds spent executing the program. Float64Histogram
77+
input.cel.program.cel.duration the time in seconds spent processing the CEL program. Float64Histogram
78+
input.cel.program.publish.duration the time in seconds spent publishing in the program. Float64Histogram
7879
7980
HTTP metrics are generated by the OTEL SDK through wrapping the transport and are scoped in the OTEL metrics as
8081
' go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp'.

‎x-pack/filebeat/otel/exporter_factory.go‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,21 @@ func NewMetricsExporterFactory(exporterOptions MetricExporterOptions) *MetricsEx
111111
func (ef *MetricsExporterFactory) GetExporter(ctx context.Context, global bool) (sdkmetric.Exporter, ExporterType, error) {
112112
exporterType := GetExporterTypeFromEnv()
113113
var err error
114-
if global && ef.globalMetricsExporter != nil {
115-
return ef.globalMetricsExporter, exporterType, nil
116-
}
114+
117115
if global {
118116
ef.lock.Lock()
119117
}
118+
119+
defer func() {
120+
if global {
121+
ef.lock.Unlock()
122+
}
123+
}()
124+
125+
if global && ef.globalMetricsExporter != nil {
126+
return ef.globalMetricsExporter, exporterType, nil
127+
}
128+
120129
var exporter sdkmetric.Exporter
121130
switch exporterType {
122131
case console:
@@ -134,7 +143,6 @@ func (ef *MetricsExporterFactory) GetExporter(ctx context.Context, global bool)
134143

135144
if global {
136145
ef.globalMetricsExporter = exporter
137-
ef.lock.Unlock()
138146
}
139147
return exporter, exporterType, err
140148
}

‎x-pack/filebeat/otel/exporter_factory_test.go‎

Lines changed: 43 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,27 @@ package otel
66

77
import (
88
"context"
9-
"os"
109
"testing"
10+
11+
"golang.org/x/sync/errgroup"
1112
)
1213

14+
func TestExporterFactoryRace(t *testing.T) {
15+
t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "set")
16+
factory := NewMetricsExporterFactory(GetDefaultMetricExporterOptions())
17+
g, ctx := errgroup.WithContext(t.Context())
18+
19+
for range 100 {
20+
g.Go(func() error {
21+
_, _, err := factory.GetExporter(ctx, true) // global=true
22+
return err
23+
})
24+
}
25+
if err := g.Wait(); err != nil {
26+
t.Errorf("GetExporter returned error: %v", err)
27+
}
28+
}
29+
1330
func TestGetGlobalExporterFactory(t *testing.T) {
1431
// set global to false
1532
factory1 := GetGlobalMetricsExporterFactory()
@@ -24,7 +41,7 @@ func TestGetGlobalExporterFactory(t *testing.T) {
2441
}
2542

2643
func TestExporterFactory(t *testing.T) {
27-
os.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "set")
44+
t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "set")
2845
options := GetDefaultMetricExporterOptions()
2946
factory := NewMetricsExporterFactory(options)
3047
if factory == nil {
@@ -72,7 +89,7 @@ func TestExporterFactory(t *testing.T) {
7289
}
7390

7491
func TestExporterFactoryNoMetricsEnvironment(t *testing.T) {
75-
os.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "set")
92+
t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "set")
7693
options := GetDefaultMetricExporterOptions()
7794
factory := NewMetricsExporterFactory(options)
7895
if factory == nil {
@@ -94,7 +111,7 @@ func TestExporterFactoryNoMetricsEnvironment(t *testing.T) {
94111
}
95112

96113
func TestGetGlobalExporterNoneType(t *testing.T) {
97-
os.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "set")
114+
t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "set")
98115
factory := GetGlobalMetricsExporterFactory()
99116
exporter, _, err := factory.GetExporter(context.Background(), true)
100117
if err != nil {
@@ -109,7 +126,7 @@ func TestGetGlobalExporterNoneType(t *testing.T) {
109126
}
110127

111128
func TestGetGlobalExporterGRPCType(t *testing.T) {
112-
os.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "set")
129+
t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "set")
113130
factory := GetGlobalMetricsExporterFactory()
114131
exporter, _, err := factory.GetExporter(context.Background(), true)
115132
if err != nil {
@@ -155,31 +172,27 @@ func TestGetExporterFromEnvironment(t *testing.T) {
155172
}
156173

157174
for _, tc := range tests {
158-
if tc.metricsExporter == "" {
159-
os.Unsetenv("OTEL_METRICS_EXPORTER")
160-
} else {
161-
os.Setenv("OTEL_METRICS_EXPORTER", tc.metricsExporter)
162-
}
163-
if tc.endpoint == "" {
164-
os.Unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT")
165-
} else {
166-
os.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", tc.endpoint)
167-
}
168-
if tc.protocol == "" {
169-
os.Unsetenv("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL")
170-
} else {
171-
os.Setenv("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL", tc.protocol)
172-
}
173-
exporter, etype, err := factory.GetExporter(context.Background(), false)
174-
if err != nil {
175-
t.Errorf("%s: GetExporter returned error: %v", tc.name, err)
176-
}
177-
if (exporter == nil) != tc.isNil {
178-
t.Errorf("%s, exporter unexpected: got nil=%v, want nil=%v", tc.name, exporter == nil, tc.isNil)
179-
}
180-
if etype != ExporterType(tc.eType) {
181-
t.Errorf("%s, exporter type unexpected: got %v, want %v", tc.name, etype, ExporterType(tc.eType))
182-
}
175+
t.Run(tc.name, func(t *testing.T) {
176+
if tc.metricsExporter != "" {
177+
t.Setenv("OTEL_METRICS_EXPORTER", tc.metricsExporter)
178+
}
179+
if tc.endpoint != "" {
180+
t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", tc.endpoint)
181+
}
182+
if tc.protocol != "" {
183+
t.Setenv("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL", tc.protocol)
184+
}
185+
exporter, etype, err := factory.GetExporter(context.Background(), false)
186+
if err != nil {
187+
t.Errorf("%s: GetExporter returned error: %v", tc.name, err)
188+
}
189+
if (exporter == nil) != tc.isNil {
190+
t.Errorf("%s, exporter unexpected: got nil=%v, want nil=%v", tc.name, exporter == nil, tc.isNil)
191+
}
192+
if etype != ExporterType(tc.eType) {
193+
t.Errorf("%s, exporter type unexpected: got %v, want %v", tc.name, etype, ExporterType(tc.eType))
194+
}
195+
})
183196

184197
}
185198
}

0 commit comments

Comments
 (0)