x/telemetry/config: start collecting counters from delve

[aarzilli]

Summary

Adds delve's counter to the telemetry configuration. There are two counters, one for crashes and one for panics we catch to avoid crashing.

This is a follow up to the approved proposal #68384

Proposed Config Change

https://go-review.googlesource.com/c/telemetry/+/655415

[gopherbot]

Change https://go.dev/cl/655415 mentions this issue: internal/chartconfig: collect counters for Delve

[aarzilli]

cc @golang/tools-team
cc @derekparker

[gabyhelp]

Related Issues

• x/telemetry: expand the scope of Go Telemetry to include Delve #68384 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[findleyr]

We're OK with the proposed counters. However, before accepting, I was skimming the implementation. It looks like sometimes the Bug counter is incremented too early? (or I'm missing something). For example, in the following case, shouldn't the Bug.Inc go in the err != nil block?

https://github.com/go-delve/delve/blob/a80904ca1f5da4ee99053c46b35f4d6a2124f824/pkg/proc/eval.go#L1053

(I am just wary of producing a large amount of unique stack data by accident).

[aarzilli]

You are right: go-delve/delve#3967

[findleyr]

Thanks. I just skimmed usage and it seems good now. Can we please update the minimum counter version to be after the fix?

[aarzilli]

Thanks. I just skimmed usage and it seems good now. Can we please update the minimum counter version to be after the fix?

I think I already did it? Delve 1.24.2 was released after that PR was merged.

[findleyr]

Thank you for confirming! Then this proposal is approved, and I will merge your CL.

[gopherbot]

Change https://go.dev/cl/673235 mentions this issue: internal/chartconfig: add paddings for Delve and update tests