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

Fix: Nil value for db.name attribute #744

Merged

Conversation

robertlaurin
Copy link
Contributor

One of our users reported some log spam during migrations on their application.

It looks like we're expecting a db.name to be present when there might not be one available.

E, [2021-05-05T15:02:17.437321 #55630] ERROR -- : OpenTelemetry error: invalid span attribute value type NilClass for key 'db.name' on span 'set names'
E, [2021-05-05T15:02:17.440246 #55630] ERROR -- : OpenTelemetry error: invalid span attribute value type NilClass for key 'db.name' on span 'create database'

@robertlaurin robertlaurin force-pushed the fix-mysql-attribute-errors branch from c1b1e3a to 15ce086 Compare May 6, 2021 14:12
Copy link
Contributor

@ericmustin ericmustin left a comment

Choose a reason for hiding this comment

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

could be good to add a test to prevent a regression but generally lgtm

@fbogsany fbogsany merged commit 94b4207 into open-telemetry:main May 7, 2021
@robertlaurin robertlaurin deleted the fix-mysql-attribute-errors branch May 7, 2021 19:51
@robertlaurin
Copy link
Contributor Author

@ericmustin

could be good to add a test to prevent a regression but generally lgtm

Hey yeah, sorry responding post merge. I agree, I find testing for the absence of a log kind of awkward and flakey as a form of test. It's possible I'm not seeing a more obvious reliable way of testing this change.

If you have any ideas here I can definitely add the test in a follow up PR.

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

Successfully merging this pull request may close these issues.

3 participants