apm

apm copied to clipboard

The span_frames_min_duration seems to be used to decide whether or not to collect stack traces for a span. The problem is that this information only becomes clear after reading the full documentation, as the name of the property does not make this easy to guess. It would be better if the name of the property was clear enough for the user to know what it is used for. Maybe stack_trace_span_min_duration, for example.

Spec issue

• [x] https://github.com/elastic/apm/issues/528

Agent issues

• [x] https://github.com/elastic/apm-agent-java/issues/2206
• [x] https://github.com/elastic/apm-agent-dotnet/issues/1529
• [x] https://github.com/elastic/apm-agent-go/issues/1142
• [x] https://github.com/elastic/apm-agent-php/issues/568
• [ ] https://github.com/elastic/apm-agent-ruby/issues/1174
• [x] https://github.com/elastic/apm-agent-nodejs/issues/2381
• [x] https://github.com/elastic/apm-agent-python/issues/1369

I agree this is a confusing name. The problem is that it has to be aligned with the other Elastic APM agents. But that doesn't mean we can't change it 🙂

How do you like span_stack_trace_min_duration?

Much better! When I read the property span_stack_trace_min_duration, I know exactly what it will be used for.

If we do this, I'd really like to also switch the meaning of the special values 0 and -1. Currently, setting the value to 0 means "never", and setting the value to -1 means "always". If you consider that 0 is a valid duration, then it's logical that duration>0 should always capture a stack trace. Therefore we really only need -1 as a special case of "never". We could leave the values for the existing config name alone.

I have also found this very confusing +1 to switch the semantics of 0 and -1

Swinging by from the future with some data -- the Node.js agent is about to merge our implementation of the ELASTIC_APM_SPAN_FRAMES_MIN_DURATION env variable. We aligned with the Java, Ruby, Python and .NET agents in implementing a "-1 means all, 0 means none" behavior, and we're keeping (for now) a legacy boolean that can turn this feature on/off wholesale.

We'd also be keenly interested in a new name for this variable and treating 0 as a valid duration.

If we go with both new name and new meaning, we can keep backwards compatibility. I still think it's a decent idea 👍

SGTM. Any volunteers to draft a spec?

I could do it, though it won't be for a week or two while I play catch-up.

Spec in #454

Reopened to use as meta issue