-
Notifications
You must be signed in to change notification settings - Fork 628
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 NonRecordSpan.attributes access #1377
Conversation
Seems like this was added in #1230, any thoughts @TheAnshul756? |
You are correct, this throws an error in the absence of SDK. Please use |
I tried that locally and it made some other test fail, we'll see what CI says |
Upd: Span detach was done in patched |
Hello @adriangb |
Sure or just merge that and I’ll rebase, either way |
Please merge this PR. |
fix span detech issue.
Done |
Hi is there anything missing to push this forward? |
I think just lint fix and changelog are remaining. |
I'm seeing this fail in my test suite:
And I can tell that
NotRecordingSpan
does not haveattributes
and it seems like that would be checked for. I tried to add a test but with all of the mocking and lack of typing I can't tell what is actually being tested where.