-
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
addiing parallelism to speed up performace issue #6111 #6139
addiing parallelism to speed up performace issue #6111 #6139
Conversation
… logger.go LogMatcher function Signed-off-by: Insomniac2904 <[email protected]>
fix function name in comment Signed-off-by: cangqiaoyuzhuo <[email protected]> Signed-off-by: Insomniac2904 <[email protected]>
## Which problem is this PR solving? The `component.UseLocalHostAsDefaultHost` gate has been removed from otel collector since v0.112.0. The example failed to start `jaeger` because of the invalid argument. ## Description of the changes - Remove the invalid command line argument. - Set endpoints for `otlpreceiver` explicitly. ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [ ] I have added unit tests for the new functionality - [ ] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: haoqixu <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: Insomniac2904 <[email protected]>
540e45f
to
393a43b
Compare
var LogMatcher = func(occurrences int, subStr string, logs []string) (bool, string) { | ||
errMsg := fmt.Sprintf("subStr '%s' does not occur %d time(s) in %v", subStr, occurrences, logs) | ||
if len(logs) < occurrences { | ||
return false, errMsg | ||
} | ||
|
||
// Count occurrences in parallel |
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.
why?
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.
Initially I thought that as log volume increases , concurrent checking for the occurrences would help. But now that I checked the function calls I see that none of the calls affect the testing time of the command that was mentioned in the issue. I have removed the changes to my code locally . Should I submit a new PR? Are there any more changes that I need to do ?
Thanks.
Please read the ticket description carefully. |
Which problem is this PR solving?
Description of the changes
t.parallel()
in test functions and used goroutines for checking occurrences in LogMatcher function.How was this change tested?
GOMAXPROCS=1 go test -parallel 128 -p 16 -json ./... | go run github.com/roblaszczak/vgt@latest
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test
Results