Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[exporter/clickhouse] Fix Nil Pointer Exception on Metrics/Traces export without service.name Resource Attribute #37034

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Fiery-Fenix
Copy link
Contributor

Description

Fixing Nil Pointer Exception regression introduced in #35725

Link to tracking issue

Fixes #37030

Testing

Unit test adjusted to include test Metric without service.name Resource Attributes

Documentation

@@ -109,7 +109,10 @@ func (g *gaugeMetrics) insert(ctx context.Context, db *sql.DB) error {
}()

for _, model := range g.gaugeModels {
serviceName, _ := model.metadata.ResAttr.Get(conventions.AttributeServiceName)
var serviceName string
if v, ok := model.metadata.ResAttr.Get(conventions.AttributeServiceName); ok {
Copy link
Member

@JaredTan95 JaredTan95 Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer it as a helper function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!
I've a made a helper function for it and also put some tests for this new function.
As a bonus - made some small optimizations on metrics export

Small optimizations on metrics export
Copy link
Member

@SpencerTorres SpencerTorres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I appreciate you updating the code with the helper function.

Thanks for fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[exporter/clickhouse] Nil pointer exception on Metrics without service.name Resource Attributes
4 participants