Skip to content

Commit 53695fb

Browse files
authored
Add nop parser for exporters (open-telemetry#3125)
* add nop parser for exporter use * chlog * Fix exporter tests * another one
1 parent 0cafedf commit 53695fb

File tree

11 files changed

+170
-18
lines changed

11 files changed

+170
-18
lines changed

.chloggen/fix-parsing-bug.yaml

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
2+
change_type: bug_fix
3+
4+
# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
5+
component: collector
6+
7+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
8+
note: Fixes a bug where an exporter would cause a port collision
9+
10+
# One or more tracking issues related to the change
11+
issues: [3124]
12+
13+
# (Optional) One or more lines of additional information to render under the primary note.
14+
# These lines will be padded with 2 spaces and then inserted directly into the document.
15+
# Use pipe (|) for multiline entries.
16+
subtext:

apis/v1beta1/config.go

+1
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ type Config struct {
141141

142142
// getPortsForComponentKinds gets the ports for the given ComponentKind(s).
143143
func (c *Config) getPortsForComponentKinds(logger logr.Logger, componentKinds ...ComponentKind) ([]corev1.ServicePort, error) {
144+
144145
var ports []corev1.ServicePort
145146
enabledComponents := c.GetEnabledComponents()
146147
for _, componentKind := range componentKinds {

apis/v1beta1/config_test.go

+1-14
Original file line numberDiff line numberDiff line change
@@ -497,25 +497,12 @@ func TestConfig_GetExporterPorts(t *testing.T) {
497497
Name: "prometheus",
498498
Port: 8889,
499499
},
500-
{
501-
Name: "otlp",
502-
Port: 4317,
503-
},
504-
{
505-
Name: "zipkin",
506-
Port: 9411,
507-
},
508500
},
509501
},
510502
{
511503
name: "extensions",
512504
file: "testdata/otelcol-extensions.yaml",
513-
want: []v1.ServicePort{
514-
{
515-
Name: "otlp-auth",
516-
Port: 4317,
517-
},
518-
},
505+
want: nil,
519506
},
520507
{
521508
name: "filelog",

controllers/reconcile_test.go

+36
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ func TestOpenTelemetryCollectorReconciler_Reconcile(t *testing.T) {
9999
deploymentExtraPorts.Annotations = map[string]string{
100100
"new-annotation": "new-value",
101101
}
102+
baseOTLPParams := testCollectorAssertNoErr(t, "test-otlp", "", otlpTestFile)
102103
ingressParams := testCollectorAssertNoErr(t, "test-ingress", "", testFileIngress)
103104
ingressParams.Spec.Ingress.Type = "ingress"
104105
updatedIngressParams := testCollectorAssertNoErr(t, "test-ingress", "", testFileIngress)
@@ -220,6 +221,41 @@ func TestOpenTelemetryCollectorReconciler_Reconcile(t *testing.T) {
220221
},
221222
},
222223
},
224+
225+
{
226+
name: "otlp receiver collector",
227+
args: args{
228+
params: baseOTLPParams,
229+
updates: []v1alpha1.OpenTelemetryCollector{},
230+
},
231+
want: []want{
232+
{
233+
result: controllerruntime.Result{},
234+
checks: []check[v1alpha1.OpenTelemetryCollector]{
235+
func(t *testing.T, params v1alpha1.OpenTelemetryCollector) {
236+
d := appsv1.StatefulSet{}
237+
exists, err := populateObjectIfExists(t, &d, namespacedObjectName(naming.Collector(params.Name), params.Namespace))
238+
assert.NoError(t, err)
239+
assert.True(t, exists)
240+
assert.Equal(t, int32(1), *d.Spec.Replicas)
241+
svc := &v1.Service{}
242+
exists, err = populateObjectIfExists(t, svc, namespacedObjectName(naming.Service(params.Name), params.Namespace))
243+
assert.NoError(t, err)
244+
assert.True(t, exists)
245+
assert.Equal(t, svc.Spec.Selector, map[string]string{
246+
"app.kubernetes.io/component": "opentelemetry-collector",
247+
"app.kubernetes.io/instance": "default.test-otlp",
248+
"app.kubernetes.io/managed-by": "opentelemetry-operator",
249+
"app.kubernetes.io/part-of": "opentelemetry",
250+
})
251+
assert.Len(t, svc.Spec.Ports, 4)
252+
},
253+
},
254+
wantErr: assert.NoError,
255+
validateErr: assert.NoError,
256+
},
257+
},
258+
},
223259
{
224260
name: "invalid mode",
225261
args: args{

controllers/suite_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ const (
9191
promFile = "testdata/test.yaml"
9292
updatedPromFile = "testdata/test_ta_update.yaml"
9393
testFileIngress = "testdata/ingress_testdata.yaml"
94+
otlpTestFile = "testdata/otlp_test.yaml"
9495
)
9596

9697
var _ autodetect.AutoDetect = (*mockAutoDetect)(nil)

controllers/testdata/otlp_test.yaml

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
receivers:
2+
otlp:
3+
protocols:
4+
grpc:
5+
http:
6+
processors:
7+
exporters:
8+
otlp:
9+
endpoint: jaeger-allinone-collector-headless.chainsaw-otlp-metrics.svc:4317
10+
tls:
11+
insecure: true
12+
prometheus:
13+
endpoint: 0.0.0.0:8889
14+
resource_to_telemetry_conversion:
15+
enabled: true # by default resource attributes are dropped
16+
service:
17+
pipelines:
18+
traces:
19+
receivers: [otlp]
20+
exporters: [otlp]
21+
metrics:
22+
receivers: [otlp]
23+
exporters: [prometheus]

internal/components/exporters/helpers.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func ParserFor(name string) components.ComponentPortParser {
3838
return parser
3939
}
4040
// We want the default for exporters to fail silently.
41-
return components.NewSilentSinglePortParser(components.ComponentType(name), components.UnsetPort)
41+
return components.NewNopParser(components.ComponentType(name), components.UnsetPort)
4242
}
4343

4444
var (

internal/components/exporters/helpers_test.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@ func TestParserForReturns(t *testing.T) {
3434
"endpoint": "localhost:9000",
3535
})
3636
assert.NoError(t, err)
37-
assert.Len(t, ports, 1)
38-
assert.Equal(t, ports[0].Port, int32(9000))
37+
assert.Len(t, ports, 0) // Should use the nop parser
3938
}
4039

4140
func TestCanRegister(t *testing.T) {

internal/components/multi_endpoint.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ func (m *MultiPortReceiver) Ports(logger logr.Logger, name string, config interf
5353
port := defaultSvc.Port
5454
if ec != nil {
5555
port = ec.GetPortNumOrDefault(logger, port)
56-
defaultSvc.Name = naming.PortName(fmt.Sprintf("%s-%s", name, protocol), port)
5756
}
57+
defaultSvc.Name = naming.PortName(fmt.Sprintf("%s-%s", name, protocol), port)
5858
ports = append(ports, ConstructServicePort(defaultSvc, port))
5959
} else {
6060
return nil, fmt.Errorf("unknown protocol set: %s", protocol)

internal/components/nop_parser.go

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright The OpenTelemetry Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package components
16+
17+
import (
18+
"fmt"
19+
20+
"github.com/go-logr/logr"
21+
corev1 "k8s.io/api/core/v1"
22+
)
23+
24+
var (
25+
_ ComponentPortParser = &NopParser{}
26+
)
27+
28+
// SingleEndpointParser is a special parser for a generic receiver that has an endpoint or listen_address in its
29+
// configuration. It doesn't self-register and should be created/used directly.
30+
type NopParser struct {
31+
name string
32+
}
33+
34+
func (n *NopParser) Ports(logger logr.Logger, name string, config interface{}) ([]corev1.ServicePort, error) {
35+
return nil, nil
36+
}
37+
38+
func (n *NopParser) ParserType() string {
39+
return ComponentType(n.name)
40+
}
41+
42+
func (n *NopParser) ParserName() string {
43+
return fmt.Sprintf("__%s", n.name)
44+
}
45+
46+
func NewNopParser(name string, port int32, opts ...PortBuilderOption) *NopParser {
47+
return &NopParser{name: name}
48+
}

internal/manifests/collector/service_test.go

+41
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,47 @@ func TestDesiredService(t *testing.T) {
226226
assert.Equal(t, expected, *actual)
227227
})
228228

229+
t.Run("should return service with OTLP ports", func(t *testing.T) {
230+
params := manifests.Params{
231+
Config: config.Config{},
232+
Log: logger,
233+
OtelCol: v1beta1.OpenTelemetryCollector{
234+
Spec: v1beta1.OpenTelemetryCollectorSpec{Config: v1beta1.Config{
235+
Receivers: v1beta1.AnyConfig{
236+
Object: map[string]interface{}{
237+
"otlp": map[string]interface{}{
238+
"protocols": map[string]interface{}{
239+
"grpc": nil,
240+
"http": nil,
241+
},
242+
},
243+
},
244+
},
245+
Exporters: v1beta1.AnyConfig{
246+
Object: map[string]interface{}{
247+
"otlp": map[string]interface{}{
248+
"endpoint": "jaeger-allinone-collector-headless.chainsaw-otlp-metrics.svc:4317",
249+
},
250+
},
251+
},
252+
Service: v1beta1.Service{
253+
Pipelines: map[string]*v1beta1.Pipeline{
254+
"traces": {
255+
Receivers: []string{"otlp"},
256+
Exporters: []string{"otlp"},
257+
},
258+
},
259+
},
260+
}},
261+
},
262+
}
263+
264+
actual, err := Service(params)
265+
assert.NotNil(t, actual)
266+
assert.Len(t, actual.Spec.Ports, 2)
267+
assert.NoError(t, err)
268+
})
269+
229270
}
230271

231272
func TestHeadlessService(t *testing.T) {

0 commit comments

Comments
 (0)