Skip to content

Commit 932cd3b

Browse files
Fiery-Fenixzeck-ops
authored andcommitted
[exporter/clickhouse] Fix Nil Pointer Exception on Metrics/Traces export without service.name Resource Attribute (open-telemetry#37034)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Fixing Nil Pointer Exception regression introduced in open-telemetry#35725 <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#37030 <!--Describe what testing was performed and which tests were added.--> #### Testing Unit test adjusted to include test Metric without `service.name` Resource Attributes <!--Describe the documentation added.--> #### Documentation <!--Please delete paragraphs that you did not use before submitting.-->
1 parent a4fecf8 commit 932cd3b

11 files changed

+94
-34
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
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: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: clickhouseexporter
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Fix Nil Pointer Exception on Metrics/Traces export without service.name Resource Attribute
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [37030]
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+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [user]

exporter/clickhouseexporter/exporter_logs.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
_ "github.com/ClickHouse/clickhouse-go/v2" // For register database driver.
1313
"go.opentelemetry.io/collector/component"
1414
"go.opentelemetry.io/collector/pdata/plog"
15-
conventions "go.opentelemetry.io/collector/semconv/v1.27.0"
1615
"go.uber.org/zap"
1716

1817
"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/clickhouseexporter/internal"
@@ -77,10 +76,7 @@ func (e *logsExporter) pushLogsData(ctx context.Context, ld plog.Logs) error {
7776
res := logs.Resource()
7877
resURL := logs.SchemaUrl()
7978
resAttr := internal.AttributesToMap(res.Attributes())
80-
var serviceName string
81-
if v, ok := res.Attributes().Get(conventions.AttributeServiceName); ok {
82-
serviceName = v.Str()
83-
}
79+
serviceName := internal.GetServiceName(res.Attributes())
8480

8581
for j := 0; j < logs.ScopeLogs().Len(); j++ {
8682
rs := logs.ScopeLogs().At(j).LogRecords()

exporter/clickhouseexporter/exporter_metrics_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ func simpleMetrics(count int) pmetric.Metrics {
294294
}
295295

296296
rm = metrics.ResourceMetrics().AppendEmpty()
297-
rm.Resource().Attributes().PutStr("service.name", "demo 2")
297+
// Removed service.name from second metric to test both with/without ServiceName cases
298298
rm.Resource().Attributes().PutStr("Resource Attributes 2", "value2")
299299
rm.Resource().SetDroppedAttributesCount(20)
300300
rm.SetSchemaUrl("Resource SchemaUrl 2")

exporter/clickhouseexporter/exporter_traces.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/ClickHouse/clickhouse-go/v2/lib/column"
1515
"go.opentelemetry.io/collector/component"
1616
"go.opentelemetry.io/collector/pdata/ptrace"
17-
conventions "go.opentelemetry.io/collector/semconv/v1.27.0"
1817
"go.uber.org/zap"
1918

2019
"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/clickhouseexporter/internal"
@@ -77,7 +76,8 @@ func (e *tracesExporter) pushTraceData(ctx context.Context, td ptrace.Traces) er
7776
spans := td.ResourceSpans().At(i)
7877
res := spans.Resource()
7978
resAttr := internal.AttributesToMap(res.Attributes())
80-
serviceName, _ := res.Attributes().Get(conventions.AttributeServiceName)
79+
serviceName := internal.GetServiceName(res.Attributes())
80+
8181
for j := 0; j < spans.ScopeSpans().Len(); j++ {
8282
rs := spans.ScopeSpans().At(j).Spans()
8383
scopeName := spans.ScopeSpans().At(j).Scope().Name()
@@ -96,7 +96,7 @@ func (e *tracesExporter) pushTraceData(ctx context.Context, td ptrace.Traces) er
9696
r.TraceState().AsRaw(),
9797
r.Name(),
9898
r.Kind().String(),
99-
serviceName.AsString(),
99+
serviceName,
100100
resAttr,
101101
scopeName,
102102
scopeVersion,

exporter/clickhouseexporter/internal/exponential_histogram_metrics.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
"go.opentelemetry.io/collector/pdata/pcommon"
1313
"go.opentelemetry.io/collector/pdata/pmetric"
14-
conventions "go.opentelemetry.io/collector/semconv/v1.27.0"
1514
"go.uber.org/zap"
1615
)
1716

@@ -130,20 +129,22 @@ func (e *expHistogramMetrics) insert(ctx context.Context, db *sql.DB) error {
130129
}()
131130

132131
for _, model := range e.expHistogramModels {
133-
serviceName, _ := model.metadata.ResAttr.Get(conventions.AttributeServiceName)
132+
resAttr := AttributesToMap(model.metadata.ResAttr)
133+
scopeAttr := AttributesToMap(model.metadata.ScopeInstr.Attributes())
134+
serviceName := GetServiceName(model.metadata.ResAttr)
134135

135136
for i := 0; i < model.expHistogram.DataPoints().Len(); i++ {
136137
dp := model.expHistogram.DataPoints().At(i)
137138
attrs, times, values, traceIDs, spanIDs := convertExemplars(dp.Exemplars())
138139
_, err = statement.ExecContext(ctx,
139-
AttributesToMap(model.metadata.ResAttr),
140+
resAttr,
140141
model.metadata.ResURL,
141142
model.metadata.ScopeInstr.Name(),
142143
model.metadata.ScopeInstr.Version(),
143-
AttributesToMap(model.metadata.ScopeInstr.Attributes()),
144+
scopeAttr,
144145
model.metadata.ScopeInstr.DroppedAttributesCount(),
145146
model.metadata.ScopeURL,
146-
serviceName.AsString(),
147+
serviceName,
147148
model.metricName,
148149
model.metricDescription,
149150
model.metricUnit,

exporter/clickhouseexporter/internal/gauge_metrics.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
"go.opentelemetry.io/collector/pdata/pcommon"
1313
"go.opentelemetry.io/collector/pdata/pmetric"
14-
conventions "go.opentelemetry.io/collector/semconv/v1.27.0"
1514
"go.uber.org/zap"
1615
)
1716

@@ -109,20 +108,22 @@ func (g *gaugeMetrics) insert(ctx context.Context, db *sql.DB) error {
109108
}()
110109

111110
for _, model := range g.gaugeModels {
112-
serviceName, _ := model.metadata.ResAttr.Get(conventions.AttributeServiceName)
111+
resAttr := AttributesToMap(model.metadata.ResAttr)
112+
scopeAttr := AttributesToMap(model.metadata.ScopeInstr.Attributes())
113+
serviceName := GetServiceName(model.metadata.ResAttr)
113114

114115
for i := 0; i < model.gauge.DataPoints().Len(); i++ {
115116
dp := model.gauge.DataPoints().At(i)
116117
attrs, times, values, traceIDs, spanIDs := convertExemplars(dp.Exemplars())
117118
_, err = statement.ExecContext(ctx,
118-
AttributesToMap(model.metadata.ResAttr),
119+
resAttr,
119120
model.metadata.ResURL,
120121
model.metadata.ScopeInstr.Name(),
121122
model.metadata.ScopeInstr.Version(),
122-
AttributesToMap(model.metadata.ScopeInstr.Attributes()),
123+
scopeAttr,
123124
model.metadata.ScopeInstr.DroppedAttributesCount(),
124125
model.metadata.ScopeURL,
125-
serviceName.AsString(),
126+
serviceName,
126127
model.metricName,
127128
model.metricDescription,
128129
model.metricUnit,

exporter/clickhouseexporter/internal/histogram_metrics.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
"go.opentelemetry.io/collector/pdata/pcommon"
1313
"go.opentelemetry.io/collector/pdata/pmetric"
14-
conventions "go.opentelemetry.io/collector/semconv/v1.27.0"
1514
"go.uber.org/zap"
1615
)
1716

@@ -121,20 +120,22 @@ func (h *histogramMetrics) insert(ctx context.Context, db *sql.DB) error {
121120
}()
122121

123122
for _, model := range h.histogramModel {
124-
serviceName, _ := model.metadata.ResAttr.Get(conventions.AttributeServiceName)
123+
resAttr := AttributesToMap(model.metadata.ResAttr)
124+
scopeAttr := AttributesToMap(model.metadata.ScopeInstr.Attributes())
125+
serviceName := GetServiceName(model.metadata.ResAttr)
125126

126127
for i := 0; i < model.histogram.DataPoints().Len(); i++ {
127128
dp := model.histogram.DataPoints().At(i)
128129
attrs, times, values, traceIDs, spanIDs := convertExemplars(dp.Exemplars())
129130
_, err = statement.ExecContext(ctx,
130-
AttributesToMap(model.metadata.ResAttr),
131+
resAttr,
131132
model.metadata.ResURL,
132133
model.metadata.ScopeInstr.Name(),
133134
model.metadata.ScopeInstr.Version(),
134-
AttributesToMap(model.metadata.ScopeInstr.Attributes()),
135+
scopeAttr,
135136
model.metadata.ScopeInstr.DroppedAttributesCount(),
136137
model.metadata.ScopeURL,
137-
serviceName.AsString(),
138+
serviceName,
138139
model.metricName,
139140
model.metricDescription,
140141
model.metricUnit,

exporter/clickhouseexporter/internal/metrics_model.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/ClickHouse/clickhouse-go/v2/lib/column/orderedmap"
1818
"go.opentelemetry.io/collector/pdata/pcommon"
1919
"go.opentelemetry.io/collector/pdata/pmetric"
20+
conventions "go.opentelemetry.io/collector/semconv/v1.27.0"
2021
"go.uber.org/zap"
2122
)
2223

@@ -175,6 +176,15 @@ func AttributesToMap(attributes pcommon.Map) column.IterableOrderedMap {
175176
}, attributes.Len())
176177
}
177178

179+
func GetServiceName(resAttr pcommon.Map) string {
180+
var serviceName string
181+
if v, ok := resAttr.Get(conventions.AttributeServiceName); ok {
182+
serviceName = v.AsString()
183+
}
184+
185+
return serviceName
186+
}
187+
178188
func convertSliceToArraySet[T any](slice []T) clickhouse.ArraySet {
179189
var set clickhouse.ArraySet
180190
for _, item := range slice {

exporter/clickhouseexporter/internal/metrics_model_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/stretchr/testify/require"
1313
"go.opentelemetry.io/collector/pdata/pcommon"
1414
"go.opentelemetry.io/collector/pdata/pmetric"
15+
conventions "go.opentelemetry.io/collector/semconv/v1.27.0"
1516
"go.uber.org/zap/zaptest"
1617
)
1718

@@ -225,3 +226,24 @@ func Test_newPlaceholder(t *testing.T) {
225226
expectStr := "(?,?,?,?,?),"
226227
require.Equal(t, newPlaceholder(5), &expectStr)
227228
}
229+
230+
func Test_GetServiceName(t *testing.T) {
231+
t.Run("should return empty string on unset service.name", func(t *testing.T) {
232+
require.Equal(t, "", GetServiceName(pcommon.NewMap()))
233+
})
234+
t.Run("should return correct string from service.name", func(t *testing.T) {
235+
resAttr := pcommon.NewMap()
236+
resAttr.PutStr(conventions.AttributeServiceName, "test-service")
237+
require.Equal(t, "test-service", GetServiceName(resAttr))
238+
})
239+
t.Run("should return empty string on empty service.name", func(t *testing.T) {
240+
resAttr := pcommon.NewMap()
241+
resAttr.PutEmpty(conventions.AttributeServiceName)
242+
require.Equal(t, "", GetServiceName(resAttr))
243+
})
244+
t.Run("should return string from non-string service.name", func(t *testing.T) {
245+
resAttr := pcommon.NewMap()
246+
resAttr.PutBool(conventions.AttributeServiceName, true)
247+
require.Equal(t, "true", GetServiceName(resAttr))
248+
})
249+
}

exporter/clickhouseexporter/internal/sum_metrics.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
"go.opentelemetry.io/collector/pdata/pcommon"
1313
"go.opentelemetry.io/collector/pdata/pmetric"
14-
conventions "go.opentelemetry.io/collector/semconv/v1.27.0"
1514
"go.uber.org/zap"
1615
)
1716

@@ -113,20 +112,22 @@ func (s *sumMetrics) insert(ctx context.Context, db *sql.DB) error {
113112
}()
114113

115114
for _, model := range s.sumModel {
116-
serviceName, _ := model.metadata.ResAttr.Get(conventions.AttributeServiceName)
115+
resAttr := AttributesToMap(model.metadata.ResAttr)
116+
scopeAttr := AttributesToMap(model.metadata.ScopeInstr.Attributes())
117+
serviceName := GetServiceName(model.metadata.ResAttr)
117118

118119
for i := 0; i < model.sum.DataPoints().Len(); i++ {
119120
dp := model.sum.DataPoints().At(i)
120121
attrs, times, values, traceIDs, spanIDs := convertExemplars(dp.Exemplars())
121122
_, err = statement.ExecContext(ctx,
122-
AttributesToMap(model.metadata.ResAttr),
123+
resAttr,
123124
model.metadata.ResURL,
124125
model.metadata.ScopeInstr.Name(),
125126
model.metadata.ScopeInstr.Version(),
126-
AttributesToMap(model.metadata.ScopeInstr.Attributes()),
127+
scopeAttr,
127128
model.metadata.ScopeInstr.DroppedAttributesCount(),
128129
model.metadata.ScopeURL,
129-
serviceName.AsString(),
130+
serviceName,
130131
model.metricName,
131132
model.metricDescription,
132133
model.metricUnit,

0 commit comments

Comments
 (0)