opencensus-java icon indicating copy to clipboard operation
opencensus-java copied to clipboard

Allow setting span start/end times explicitly

Open tigrannajaryan opened this issue 6 years ago • 11 comments

We have a use case where the start/end times of traces is coming from an external system. We need to be able to explicitly set the times instead of the times automatically retrieved from the local clock.

The suggested change to the Span API is the following:

  1. Add void setStartTime(Timestamp startTimestamp) to Span.
  2. Add void end(EndSpanOptions options, Timestamp endTimestamp) to Span.

Here is the draft of proposed solution that I can turn into Pull Request if this issue is accepted:

https://github.com/census-instrumentation/opencensus-java/compare/master...tigrannajaryan:feature/tigran/span-times?expand=1

tigrannajaryan avatar May 15 '19 23:05 tigrannajaryan

Do you really need to construct a Span or you can create directly the whole SpanData?

bogdandrutu avatar May 16 '19 00:05 bogdandrutu

I am constructing Spans, I didn't consider creating SpanData directly. I can check that to see if it works for my use case.

For reference here is how I use this feature in my code currently:

Span span = tracer.spanBuilderWithExplicitParent(operationName, parentSpan).startSpan();
span.setStartTime(Timestamp.fromMicros(startTimeMicros));
span.end(EndSpanOptions.DEFAULT, Timestamp.fromMicros(endTimeMicros));

tigrannajaryan avatar May 16 '19 00:05 tigrannajaryan

@bogdandrutu I cannot see how to achieve the same end result using SpanData. I need to create spans and have them exported using OcAgentTraceExporter (which I register earlier). Can you give me some hints on how it can be done with SpanData? I see that SpanData accepts start/end timestamps directly, so that's good but I don't see how to initiate the delivery of SpanData.

tigrannajaryan avatar May 16 '19 00:05 tigrannajaryan

@tigrannajaryan you are correct, we lack that but my comment was mostly to confirm that you have everything you need if we add a RecordSpanData on the tracer similar to OpenTelemetry.

I will give a try soon.

bogdandrutu avatar May 16 '19 03:05 bogdandrutu

@bogdandrutu sounds good.I will use our fork for now.

tigrannajaryan avatar May 16 '19 18:05 tigrannajaryan

Just want to add here that my team is also very interested in this.

Our use-case is that we are tracing through a pipeline system (where each step is a different system / JVM). So for each step in the pipeline, we have a span that starts after its root span has ended. We'd like to construct a span that covers the whole of the pipeline: the start is in the first step of the pipeline and the end being the end-time of processing the last step in the pipeline.

dietervdw-spotify avatar Jul 04 '19 11:07 dietervdw-spotify

fyi I think a lot of things like this were also discussed in census repo and some before that in the telemetry-java repo. might be worthwhile linking when that's the case as eventhough this is a fresh repo it is not really a new project per se.

codefromthecrypt avatar Jul 19 '19 00:07 codefromthecrypt

To contribute a concrete use case of this, armeria server has the ability to stash a precise "real" timestamp which is before instrumentation is invoked. While most instrumentation record duration as time between instrumentation hooks, Armeria use brave's ability to overwrite the start timestamp to get the complete duration of the work.

codefromthecrypt avatar Jul 19 '19 00:07 codefromthecrypt

Eventhough this is possible in Brave, it is still errorprone and it normally makes me cringe to see manual control of timestamps as usually this creates problems. For this reason the common advice is to let the clock do its job or write data directly if the whole operation is being recorded after the fact.

codefromthecrypt avatar Jul 19 '19 00:07 codefromthecrypt

@tigrannajaryan

For reference here is how I use this feature in my code currently:

Span span = tracer.spanBuilderWithExplicitParent(operationName, parentSpan).startSpan(); span.setStartTime(Timestamp.fromMicros(startTimeMicros)); span.end(EndSpanOptions.DEFAULT, Timestamp.fromMicros(endTimeMicros));

I don't understand how your example works. Am I right that startTimeMicros and endTimeMicros are microseconds since epoch? (BTW I can only find Timestamp.fromMillis() in the current code)

At least this is my assumption. I checked out your code and tried it like you've shown it in the example, however, the timestamps in the spans are not correct.

The root cause seems to be the TimestampConverter which does not convert the Timestamp correctly: this.startNanoTime = this.timestampConverter.convertTimestamp(startTimestamp);

MahatmaFatalError avatar Jul 23 '19 15:07 MahatmaFatalError

@MahatmaFatalError sorry, I no longer have that piece of code around to dig into it. I solved my problem in a different (unrelated) way, so can't help much with this.

tigrannajaryan avatar Jul 23 '19 16:07 tigrannajaryan