-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Copy the scope_name of log entries to scopeLog #24681
Copy the scope_name of log entries to scopeLog #24681
Conversation
|
Thanks! Please add a changelog entry. |
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.
Thank you for the PR @bin-shi-mulesoft. The only problem I see is that this won't work when there are logs from multiple scopes within a single resource.
if e.ScopeName != "" { | ||
sl.Scope().SetName(e.ScopeName) | ||
} |
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.
Unfortunately, I do no think this is the technically correct solution. Say we have four logs, two each from "Resource 1" and "Resource 2", and within each pair, one log is from "Scope A" and the other from "Scope B".
- Resource 1, Scope A: "foo"
- Resource 1, Scope B: "bar"
- Resource 2, Scope A: "hello"
- Resource 2, Scope B: "world"
Current logic which groups logs by resource but ignores the scope would group these together like this:
- Resource 1: { Scope "": { "foo", "bar" } }
- Resource 2: { Scope "": { "hello", "world" } }
The correct result would group logs according to resource and scope:
- Resource 1: { Scope A: { "foo" }, Scope B: { "bar" } }
- Resource 2: { Scope A: { "hello" }, Scope B: { "world" } }
However, this behavior proposed here would result in the following, because it still only creates one scope per resource (even though it does assign the name).
- Resource 1: { Scope A: { "foo", "bar" } }
- Resource 2: { Scope A: { "hello", "world" } }
I think the solution must involve managing a set of scopes for each resource, e.g:
// map[resourceID]map[e.ScopeName]scopeIndex
scopeIndexByResource := make(map[uint64]map[string]int)
scopesInThisResource := scopeIndexByResource[resourceID]
scopeIdx, ok := scopesInThisResource[e.ScopeName]
if !ok {
scopesInThisResource[e.ScopeName] = rl.ScopeLogs().Len()
sl = rl.ScopeLogs().AppendEmpty()
sl.SetName(e.ScopeName)
} else {
sl = rl.ScopeLogs().At(scopeIdx)
}
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Description:
Bugfix: Copy the scope_name of log entries to the scopeLog
Link to tracking Issue:
#23387
Testing:
Added unit test to verify the scope name is not empty.
Documentation: