Apm: Rename the span_frames_min_duration property

Created on 23 Jan 2019  路  9Comments  路  Source: elastic/apm

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.

apm-agents agent-python

Most helpful comment

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.

All 9 comments

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

Was this page helpful?
0 / 5 - 0 ratings

Related issues

felixbarny picture felixbarny  路  10Comments

formgeist picture formgeist  路  7Comments

felixbarny picture felixbarny  路  10Comments

beniwohli picture beniwohli  路  10Comments

mikker picture mikker  路  11Comments