x/telemetry/internal/counter: TestStack fails if test function name is slightly longer

[hyangah]

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 @jamalc @rsc

[pjweinb]

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: ***@***.***>

[hyangah]

We will try

1. increase the maxNameLen (and the page size)
2. apply a simple name compression rule in counter name generation.

[gopherbot]

Change https://go.dev/cl/509175 mentions this issue: internal/counter: increase pageSize and maxNameLen