-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[ES] Add feature to stop Legacy Trace IDs handling in SpanReader #6848
Conversation
Signed-off-by: Manik2708 <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6848 +/- ##
==========================================
- Coverage 96.06% 96.03% -0.04%
==========================================
Files 367 368 +1
Lines 20787 20811 +24
==========================================
+ Hits 19969 19985 +16
- Misses 624 630 +6
- Partials 194 196 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Manik2708 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you see my comment #6848 (comment) ?
@yurishkuro Yes, but what I could understand from that comment that we need to wrap it inside |
If you're a user of jaeger-v1 (e.g. |
Signed-off-by: Manik2708 <[email protected]>
@yurishkuro I have given a try for implementing feature gates, please review! Is that what we needed? |
Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik2708 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another thing that would be good to do is to add the following to some shared function and call from different mains
settings := otelcol.CollectorSettings{}
otelCmd := otelcol.NewCommand(settings)
for _, cmd := range otelCmd.Commands() {
if cmd.Name() == "featuregate" {
otelCmd.RemoveCommand(cmd)
command.AddCommand(cmd)
}
}
@yurishkuro Ok, so this is to provide user the information about feature gates, right? I am thinking it to add in |
no, it's not ok to abuse AddFlags for that. The pattern we have is |
Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik2708 <[email protected]>
@yurishkuro If the changes look fine, then I suggest two points in
|
agreed - feel free to include in this PR |
Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik2708 <[email protected]>
@yurishkuro I tried adding featuregate command in es-index cleaner, but that is failing integration tests. It might require some changes in the tests, so we can do it in a seperate PR! |
Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik2708 <[email protected]>
Which problem is this PR solving?
Fixes a part of #6458
Description of the changes
FindTraces
andGetTrace
of SpanReader to make them reusable for v2 APIs #6845 (comment), legacy trace id is moved to feature gateHow was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:npm run lint
andnpm run test