Closes #459 - Implemented stack trace sampling #930

JonasKunz · 2020-09-24T10:49:05Z

CLsoes #459

This change is

Patrick-Eichhorn

Reviewed 30 of 30 files at r1.
Reviewable status: all files reviewed, 18 unresolved discussions (waiting on @JonasKunz)

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/CustomSpanBuilder.java, line 34 at r1 (raw file):

    /**
     * The startEndHandler member of {@link io.opencensus.implcore.trace.SpanBuilderImpl.Options}.

Comment: The timestampConverter member of (@link io.opencencus.implcore.internal.RecordEventsSpanImpl) ?

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/Invocation.java, line 61 at r1 (raw file):

     * Stored the invocation which happened directly after this invocation.
     * This implies that nextSibling has the same parent.
     * This field acts as a single linked lsit pointer for the list of children of the parent.

typo? (lsit pointer)

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/Invocation.java, line 133 at r1 (raw file):

    /**
     * To preserve clarity of exported traces, we "hide" certain invocations, meaning that they will not be exported.

Note: talk about

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/InvocationResolver.java, line 31 at r1 (raw file):

        }

        Invocation rootInvocation = new Invocation(null, null, null); //dummy object, just used as a lsit

typo list?

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/InvocationResolver.java, line 49 at r1 (raw file):

     * @param durations  an index for the method durations (computed via {@link #getMethodDurations(List)}
     * @param startDepth the depth at which we start looking for the methods within the stack traces.
     * @param output     All events from the "events" argument will be appended to this output list, but possibly ienhanced with better stack traces.

typo ienhanced?

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/InvocationResolver.java, line 140 at r1 (raw file):

                int endIndex = findLastEqualEvent(events, offset, depth);
                endIndex = extendEndToIncludeInstrumentations(events, offset, endIndex);
                if (offset != endIndex) { //at least to events have fallen into the range

typo two?

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/PlaceholderSpan.java, line 27 at r1 (raw file):

    /**
     * Stores teh attributes added to this span.

typo the

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/SampledTrace.java, line 51 at r1 (raw file):

    /**
     * In order to recosntruct the stack trace, we need to know how many frames on the stack trace should be ignored.

typo reconstruct

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/SampledTrace.java, line 53 at r1 (raw file):

     * In order to recosntruct the stack trace, we need to know how many frames on the stack trace should be ignored.
     * For this purpose we need a stack trace of the method which started the stack-trace sampling.
     * We capture the stack-trace by creating a new Throwable isntance (the constructor captures the stack trace).

typo instance

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/SampledTrace.java, line 60 at r1 (raw file):

    /**
     * The span which acts as a root for all sampled methods.

is always an instrumented method?
Where in the code is it prevented to start new threads (for stack trace sampling) with one instrumented method under another instrumented method?

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/SampledTrace.java, line 66 at r1 (raw file):

    /**
     * The list storing the sequence of events.
     * E.g. whenever a stack-trace sample is recorded, an isntrumented is entered or exited,

typo instrumented; instrumented method?

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/SampledTrace.java, line 128 at r1 (raw file):

     * @return a callback which must be invoked as soon as the method finishes.
     */
    public synchronized MethodExitNotifier newSpanStarted(PlaceholderSpan span, String className, String methodName) {

what dose the MethodExitNotifier do? Is the addExit method called instantly?

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/SampledTrace.java, line 139 at r1 (raw file):

    /**
     * Should be called when an instrumented method is called within this trace, which however does continue an exisitign span instead of creating a new one.

what means continued? When is this the case? or when is it needed?

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/SampledTrace.java, line 189 at r1 (raw file):

    }

    private void exportWithChildren(Invocation invoc, Span parentSpan) {

how exactly does this method work?

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/StackTraceSampler.java, line 147 at r1 (raw file):

    /**
     * Continues a span but makes it's children stack-trace sampling aware.

when is this needed/called?

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/config/MethodHookConfigurationResolver.java, line 80 at r1 (raw file):

            builder.errorStatus(getAndDetectConflicts(tracingRules, r -> r.getTracing()
                    .getErrorStatus(), s -> !StringUtils.isEmpty(s), "error-status data key"));
            builder.autoTracing(getAndDetectConflicts(tracingRules, r -> r.getTracing()

what happens at this point? and when :)

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/context/InspectitContextImpl.java, line 4 at r1 (raw file):

import io.grpc.Context;
import io.opencensus.common.Function;

what is this file for?

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/hook/MethodHookGenerator.java, line 14 at r1 (raw file):

import rocks.inspectit.ocelot.config.model.instrumentation.rules.MetricRecordingSettings;
import rocks.inspectit.ocelot.config.model.instrumentation.rules.RuleTracingSettings;
import rocks.inspectit.ocelot.core.instrumentation.autotracing.StackTraceSampler;

what is this file for?

Patrick-Eichhorn

Reviewed 6 of 6 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved

Henning-Schulz

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @JonasKunz)

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/InvocationResolver.java, line 70 at r2 (raw file):

                    while (currentDepth < trace.size() && !isSameMethod(trace.get(currentDepth), entryEvent)) {
                        currentDepth++;
                    } //after the loop, currentDepth is bigger than the trace OR points at the found span

If I understood correctly, currentDepth bigger than the trace means that something went wrong, as we have

entry method X --> stack trace not containing X --> exit X

It could make sense to highlight in the comment that this is a fallback (or, if I'm wrong, explain why/how it can happen).

JonasKunz

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @JonasKunz)

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/autotracing/CustomSpanBuilder.java, line 34 at r1 (raw file):