x/telemetry/internal/counter: TestStack fails if test function name is slightly longer #61181
Assignees
Labels
FrozenDueToAge
NeedsDecision
Feedback is required from experts, contributors, and/or the community before a change can be made.
telemetry
x/telemetry issues
Milestone
Comments
|
My initial reaction is that it would be better to increase maxNameLen if
this is really a problem. (fixing the test is not a real problem: don't
rename the function) In the general case each stack frame comes from a
different import path. Of course, the package, function, line number, and
punctuation come from a limited alphabet, and could be compressed in
various ways, Any fixed maxNameLen, though, suggests that stack counters
should only report a small number of frames.
…On Wed, Jul 5, 2023 at 12:26 PM Hyang-Ah Hana Kim ***@***.***> wrote:
https://github.com/golang/telemetry/blob/2375252a9134bc3b5cb502adb82264656851a7b9/internal/counter/counter_test.go#L363C7-L363C7
This function f is currently used to test the stack counter's behavior.
If we rename it to something slightly longer, e.g. recursiveFn, the test
starts to fail due to the truncated counter name.
Currently, stack counter names are constructed by concatenating full func
loc info in a call stack
https://github.com/golang/telemetry/blob/2375252a9134bc3b5cb502adb82264656851a7b9/internal/counter/stackcounter.go#L62
The function loc info is of the form <package>.<func>:<linenumber> and
the package path itself can be long, and the concatenation can easily
exceed maxNameLen (=256).
It looks like we need more efficient encoding & datastructure for the
stack info.
cc @pjweinb <https://github.com/pjweinb> @jamalc
<https://github.com/jamalc> @rsc <https://github.com/rsc>
—
Reply to this email directly, view it on GitHub
<#61181>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABJIAIZCZQVESFBINPZC7JTXOWIS3ANCNFSM6AAAAAAZ7GEAOY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
joedian
added
the
NeedsDecision
|
We will try
|
|
Change https://go.dev/cl/509175 mentions this issue: |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
https://github.com/golang/telemetry/blob/2375252a9134bc3b5cb502adb82264656851a7b9/internal/counter/counter_test.go#L363C7-L363C7
This function
fis currently used to test the stack counter's behavior.If we rename it to something slightly longer, e.g.
recursiveFn, the test starts to fail due to the truncated counter name.Currently, stack counter names are constructed by concatenating full func loc info in a call stack https://github.com/golang/telemetry/blob/2375252a9134bc3b5cb502adb82264656851a7b9/internal/counter/stackcounter.go#L62 The function loc info is of the form
<package>.<func>:<linenumber>and the package path itself can be long, and the concatenation can easily exceedmaxNameLen(=256).It looks like we need more efficient encoding & datastructure for the stack info.
cc @pjweinb @jamalc @rsc
The text was updated successfully, but these errors were encountered: