[VOTE] MP-0007 Distributed Tracing Proposal

[Steve Fontes]

Hi All,
I think the current proposal takes into account the comments that have been received.

The pull request with the content of the proposal is here:
https://github.com/eclipse/microprofile-evolution-process/pull/34

Steve Fontes

[Ondrej Mihályi]

I can't vote +1 until the proposal expresses a desire to standardize tracer interoperability besides the Java API.

As Mark Struberg commented on the original PR #27, with distributed tracing, standard way of passing tracing ID is more important than standard API to star/stop spans. The API is just a sugar on top of instrumenting client and server endpoints, which can work without any API. But with compatible instrumentation, there's no way to ensure that different services can pick up a corellation id and join the trace transparently.

As Erin Schnabel (ebulient) pointed out there are already semantic recommendations we could adopt, as well as the B3 format to save traces, so I strongly suggest that the proposal addresses them. Otherwise I think that it misses the most important aspects of tracing and I can't support it. Simply relying on using the same instrumentation across all services isn't enough and I can't imagine how we would enforce this without making decisions at development time which the proposal declares to avoid.

--Ondrej

[Ladislav Thon]

I'm no expert, but single common API seems vastly more important to me than a single common serialization format. And again, I'm no expert, but maybe trying to specify a single common serialization format is premature at this point.

I'm a big fan of a design approach of "when in doubt, leave it out". I think it's clear there's a big amount of doubt on this point.

And I know this is a vote thread, and I'm no committer, so I'll leave it at that.

LT

Dne 2. 6. 2017 23:39 napsal uživatel "Ondrej Mihályi" <ondrej....@gmail.com>:

--
You received this message because you are subscribed to the Google Groups "MicroProfile" group.
To unsubscribe from this group and stop receiving emails from it, send an email to microprofile+unsubscribe@googlegroups.com.
To post to this group, send email to microp...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/microprofile/19404380-1aa8-4aa0-aa34-7148a1648182%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[ondrej....@gmail.com]

If the API is more important in this proposal, it should simply be called “tracing” not “distributed tracing” to avoid confusion. Any API just adds more tracing points and ability to start/stop traces, but it’s not related to a distributed nature at all.

 

If we rename the proposal and drop any irrelevant references to “distributed” from it, I’d support it. I just wanted to point out that without addressing interoperability and wire protocols, it has nothing to do with distributed requests.

 

--Ondrej

 

Od: Ladislav Thon
Odesláno:sobota 3. června 2017 9:49
Komu: Ondrej Mihályi
Kopie: MicroProfile
Předmět: Re: [microprofile] [VOTE] MP-0007 Distributed Tracing Proposal

 

I'm no expert, but single common API seems vastly more important to me than a single common serialization format. And again, I'm no expert, but maybe trying to specify a single common serialization format is premature at this point.

 

I'm a big fan of a design approach of "when in doubt, leave it out". I think it's clear there's a big amount of doubt on this point.

 

And I know this is a vote thread, and I'm no committer, so I'll leave it at that.

 

LT

 

Dne 2. 6. 2017 23:39 napsal uživatel "Ondrej Mihályi" <ondrej....@gmail.com>:

I can't vote +1 until the proposal expresses a desire to standardize tracer interoperability besides the Java API.

As Mark Struberg commented on the original PR #27, with distributed tracing, standard way of passing tracing ID is more important than standard API to star/stop spans. The API is just a sugar on top of instrumenting client and server endpoints, which can work without any API. But with compatible instrumentation, there's no way to ensure that different services can pick up a corellation id and join the trace transparently.

As Erin Schnabel (ebulient) pointed out there are already semantic recommendations we could adopt, as well as the B3 format to save traces, so I strongly suggest that the proposal addresses them. Otherwise I think that it misses the most important aspects of tracing and I can't support it. Simply relying on using the same instrumentation across all services isn't enough and I can't imagine how we would enforce this without making decisions at development time which the proposal declares to avoid.

--Ondrej

--
You received this message because you are subscribed to the Google Groups "MicroProfile" group.

To unsubscribe from this group and stop receiving emails from it, send an email to microprofile...@googlegroups.com.

[plo...@redhat.com]

Hi, 

I have a couple of comments to the proposal.

@Trace - should include tags, there are OpenTracing implementations which change ID generation based on supplied tags at span start time.

relationship=[ChildOf|FollowsFrom|New] - ChildOf and FollowsFrom are fine, but what is New? Can users define their own relationships? When there is no active span there is no reference/relationship to the previous span. New is going to be used only for situations when a caller wants to force a new trace creation. In OT Java API it is named as `ignoreActiveSpan` https://github.com/opentracing/opentracing-java/blob/master/opentracing-api/src/main/java/io/opentracing/Tracer.java#L143. What I am saying is that `relationship` is a mixture of reference type and a new trace creation.

[Gary Brown]

Regarding the @Trace annotation and tags, there are two options:

1) Add 'tags' to the annotation as Pavol suggests

2) The block can use both @Trace and @TraceDecorate annotations, and the annotation processor should use the tags from the decorate annotation when building the span.

Agree with Pavol that 'New' as a relationship type may be misleading - might be better to just have a boolean annotation parameter 'ignoreActiveSpan' (default false) to more directly relate to the opentracing API.

Another point about relationship parameter - not sure it is currently necessary on the @Trace annotation, as the only meaningful value is 'ChildOf'. A 'FollowsFrom' would require a span context to be explicitly provided - so may be better that this is handled as one of the more complex scenarios where the application needs to use the Tracer directly?

Regards

Gary

--

You received this message because you are subscribed to the Google Groups "MicroProfile" group.

To unsubscribe from this group and stop receiving emails from it, send an email to microprofile+unsubscribe@googlegroups.com.

To post to this group, send email to microp...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/microprofile/6e6a767e-0b75-4c11-8fad-6b1022a85611%40googlegroups.com.

[Gary Brown]

Also forgot to mention - the PR #115 has now been merged and released, so the active span management is part of the Tracer API. So the PR ref should probably be removed.

[Ondrej Mihályi]

Hi Gary, Pavol,

I believe that your points can be discussed even later if the proposal is accepted. This is a voting thread, so please stick to voting.

I expressed my concerns only because I couldn't vote +1.

If the proposal is renamed to simply "Tracing", without any notion of distributed requests, I'm OK to vote +1. Otherwise I have to vote against because I have a completely different expectation from a "distributed" tracing proposal.

--Ondro

[Gary Brown]

Ok thanks. In which case I vote +1 on the basis that some refinement would be possible.

--
You received this message because you are subscribed to the Google Groups "MicroProfile" group.
To unsubscribe from this group and stop receiving emails from it, send an email to microprofile+unsubscribe@googlegroups.com.
To post to this group, send email to microp...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/microprofile/e4d053f9-db53-48a5-81f7-136ec2aa6c15%40googlegroups.com.

[plo...@redhat.com]

+1 on the whole proposal. 

On Tuesday, June 6, 2017 at 11:57:12 AM UTC+2, Gary Brown wrote:

Regarding the @Trace annotation and tags, there are two options:

1) Add 'tags' to the annotation as Pavol suggests

2) The block can use both @Trace and @TraceDecorate annotations, and the annotation processor should use the tags from the decorate annotation when building the span.

Agree with Pavol that 'New' as a relationship type may be misleading - might be better to just have a boolean annotation parameter 'ignoreActiveSpan' (default false) to more directly relate to the opentracing API.

Another point about relationship parameter - not sure it is currently necessary on the @Trace annotation, as the only meaningful value is 'ChildOf'. A 'FollowsFrom' would require a span context to be explicitly provided - so may be better that this is handled as one of the more complex scenarios where the application needs to use the Tracer directly?

This is not true follows from can be used (e.g. start an async task on which parent span does not depend). ChildOf is just more used and should be a default value.

 

Regards

Gary

On Mon, Jun 5, 2017 at 11:23 AM, <plo...@redhat.com> wrote:

Hi, 

I have a couple of comments to the proposal.

@Trace - should include tags, there are OpenTracing implementations which change ID generation based on supplied tags at span start time.

relationship=[ChildOf|FollowsFrom|New] - ChildOf and FollowsFrom are fine, but what is New? Can users define their own relationships? When there is no active span there is no reference/relationship to the previous span. New is going to be used only for situations when a caller wants to force a new trace creation. In OT Java API it is named as `ignoreActiveSpan` https://github.com/opentracing/opentracing-java/blob/master/opentracing-api/src/main/java/io/opentracing/Tracer.java#L143. What I am saying is that `relationship` is a mixture of reference type and a new trace creation.

On Friday, June 2, 2017 at 7:49:13 PM UTC+2, Steve Fontes wrote:

Hi All,
I think the current proposal takes into account the comments that have been received.

The pull request with the content of the proposal is here:
https://github.com/eclipse/microprofile-evolution-process/pull/34

Steve Fontes

--
You received this message because you are subscribed to the Google Groups "MicroProfile" group.

To unsubscribe from this group and stop receiving emails from it, send an email to microprofile...@googlegroups.com.

To post to this group, send email to microp...@googlegroups.com.

[Steve Fontes]

I've added some responses to the discussion going on here in the discussion thread -- https://groups.google.com/forum/#!topic/microprofile/yCEEswknl_w