Add init attributes in constructor and sync tracing to work with new … #4206
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
hey @ArtAhmetaj |
|
Pinging @dyladan, @mayurkale22 and @pichlermarc in an attempt to make this PR gain some traction. The current implementation makes it impossible to make transformations in |
Flarna
reviewed
Nov 9, 2023
Flarna
reviewed
Nov 9, 2023
|
Your tests seem to verify only the |
|
@ArtAhmetaj checking if you are picking this up. I can pick this up if you don't have the time. |
8 tasks
Add spacing on init attributes line Co-authored-by: Gerhard Stöbich <[email protected]>
Replace truthy value check with loose null check Co-authored-by: Gerhard Stöbich <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which problem is this PR solving?
This PR fixes issue #4188 in which the span processor on start method lacks attributes since they are set after with an accessor method (setAttributes) and not on the constructor itself.
Short description of the changes
The PR adds constructor attributes to the span which are optional and if they are added from this layer the spanProcessor.onStart method will have context of attributes whenever it is called, this doest not break compatibility since setAttributes is still a valid public method and also the changes needed on wrappers like the Tracer are done.
Spans have added tests on all constructor tests I could find, the current tests should validate tracer functionality correctly.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist:
This bug fix should not be noticable since the wrapper tracer call for starting a span should not change functionality (it is only the internal span constructor changing), would like input if I have to change the documentation for the Span constructor itself considering it is @deprecated.