Fixes #210 : add support for multi reference and expose references co… by hypnoce · Pull Request #213 · opentracing/opentracing-java

hypnoce · 2017-11-05T16:20:08Z

…ntext and type from MockSpan.

yurishkuro · 2017-11-05T16:28:10Z

 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;


isn't this only available from 1.7+?

Yes. I thought the compilation source and target were at least 1.7 since it is set in the pom file (set to 1.8 but main.java.version is set to 1.7). Let me know is this should be changed.

ah, ok - the api module is 1.6, the rest are 1.7, so no new change is introduced here

coveralls · 2017-11-05T16:30:12Z

Coverage increased (+1.8%) to 74.925% when pulling 9d694ef on hypnoce:multi_reference_with_reference_type into e000eb0 on opentracing:master.

yurishkuro · 2017-11-05T16:42:58Z

+        Reference preferredReference = references.get(0);
+        for (Reference reference : references) {
+            // childOf takes precedence as a preferred parent
+            if (References.CHILD_OF.equals(reference.getReferenceType())


why not just return immediately in this case?

Indeed, I don't have a strong opinion on that. The control flow is the same. I find it more readable to have a single return at the end but I will change this.

what I meant was this:

if list.isEmpty { return nil; } for r in list { if r.isChildOf { return r.context() } } return list.get(0).context();

faster and simpler

coveralls · 2017-11-05T16:51:17Z

Coverage increased (+1.8%) to 74.85% when pulling 4719773 on hypnoce:multi_reference_with_reference_type into e000eb0 on opentracing:master.

coveralls · 2017-11-05T21:57:49Z

Coverage increased (+1.6%) to 74.699% when pulling c1adb9c on hypnoce:multi_reference_with_reference_type into e000eb0 on opentracing:master.

yurishkuro · 2017-11-06T01:05:26Z

lgtm

cc @pavolloffay

pavolloffay · 2017-11-06T13:44:11Z

LGTM,

Java doc in MockSpan.parentId() has to be changed: remove todo, make a note that it returns the first childOf reference and add link to MockSpan.references().

pavolloffay · 2017-11-06T13:34:26Z

-            if (firstParent == null && (
-                    referenceType.equals(References.CHILD_OF) || referenceType.equals(References.FOLLOWS_FROM))) {
-                this.firstParent = (MockSpan.MockContext)referencedContext;
+            if (referenceType.equals(References.CHILD_OF) || referenceType.equals(References.FOLLOWS_FROM)) {


Why this check? It can support arbitrary references.

Indeed, I just kept the current behaviour. I will change this.

…ferences context and type from MockSpan.

hypnoce · 2017-11-06T21:40:08Z

@pavolloffay I made the required changes. Let me know if that looks good to you ! Thanks !

coveralls · 2017-11-06T21:41:59Z

Coverage increased (+1.6%) to 74.699% when pulling 2ff3277 on hypnoce:multi_reference_with_reference_type into e000eb0 on opentracing:master.

@deprecated

* Code cleanup (opentracing#199) - Propagate @deprecated annotation to implementations - Remove redundant "static final" definition from interface - Use Collections.emptyMap instead of Collections.EMPTY_MAP to preserve type * Add support for multi reference and expose references context and type from MockSpan. (opentracing#213) * Publish test artifact for Globaltracer testutil (opentracing#209) * Fix MockTracer.ChildOf not fail if there is a null argument passed (opentracing#226) * Make ChildOf not fail if there is a null argument passed * Moved tracer test to where it belongs. Fixed typo. * Use correct reference in Javadoc (opentracing#230) * MockTracer use text map propag in the default constructor (opentracing#179) * Implement a simpler Span propagation without lifetime handling. (opentracing#188) - Scope replaces ActiveSpan, without sharing a common ancestor with Span. - ScopeManager replaces ActiveSpanSource. - No reference-count to handle Span's lifetime. - A simple thread-local based ScopeManager. * travis publish.sh allow releases from branches (opentracing#191) * Travis allow release from non master branches (opentracing#192) * Travis allow release from non master branches Publis script compares remote branch with current checkout. This passes travis_branch into git checkout command so it will compare the same branches. * Fix comments * Travis publish script, remove -RC on git checkout (opentracing#193) * Update the Travis script to allow publishing from v.0.0.0 branches. (opentracing#195) Thing is, we cannot publish from 0.0.0-style branches as they are excluded, based on the current global Travis configuration, thus we try to publish from branches following a v.0.0.0 style, if any. * Readme document release process for release candidates (opentracing#198) * Readme document release process for release candidates * Adjust publish.sh to work with release from master branch * Add Examples for async use cases (opentracing#197) * Add examples for async as test cases This includes execution flow similar to: * Actor ask/tell * Promises with callbacks * Work interleaved on a thread using suspend/resume. The implementations of these execution models are obviously very simplistic, but intended to emphasize the tracing aspect. * Update README files to reflect the Scope concept. (opentracing#217) * Let Tracer.activeSpan() be a shorthand for ScopeManager's active Span. (opentracing#228) * Let Tracer.activeSpan() be a shorthand for ScopeManager's active Span. * Document the null case for Tracer.activeSpan(). * Have Tracer.activeSpan() return null. * Remove @link for Tracer.activeSpan(). 'Nested' links do not happen to exist under javadoc, so Tracer.scopeManager().active().span() is now not a link. * Change BINARY to be resizable and stream-oriented. (opentracing#223) * Change BINARY to be resizable and stream-oriented. * Abstract the Binary adapter and have an Adapters class to return it. * Remove the isInbound/isOutbound methods from BinaryAdapter. * Make Binary use the Channel paradigm for injection/extraction * Have binary methods in Adapters named after inject/extract. * Add a BINARY propagator for MockTracer. * Throw RuntimeException in case of errors during BINARY's injection. * Put braces around if-blocks for Adapters/BinaryAdapter. * Use verbose messages for null parameters in Adapters. * SpanBuilder deprecate startManual (opentracing#225) * SpanBuilder deprecate startManual * Fix review comments * Remove default finish behaviour for `activate()` (opentracing#219) * Do not auto finish on scope.close * Fix review comments * Fix review comments * Add explanatory statement about not auto-finishing * Define only activate(s, bool) * Use the parameterless startActive() in a forgotten test in MockTracerTest.

@deprecated

* Code cleanup (#199) - Propagate @deprecated annotation to implementations - Remove redundant "static final" definition from interface - Use Collections.emptyMap instead of Collections.EMPTY_MAP to preserve type * Add support for multi reference and expose references context and type from MockSpan. (#213) * Publish test artifact for Globaltracer testutil (#209) * Fix MockTracer.ChildOf not fail if there is a null argument passed (#226) * Make ChildOf not fail if there is a null argument passed * Moved tracer test to where it belongs. Fixed typo. * Use correct reference in Javadoc (#230) * MockTracer use text map propag in the default constructor (#179)

hypnoce mentioned this pull request Nov 5, 2017

Add tracing for entity reading and writing. opentracing-contrib/java-jaxrs#46

Merged

hypnoce force-pushed the multi_reference_with_reference_type branch from 98211a0 to 9d694ef Compare November 5, 2017 16:27

yurishkuro reviewed Nov 5, 2017

View reviewed changes

yurishkuro approved these changes Nov 5, 2017

View reviewed changes

hypnoce force-pushed the multi_reference_with_reference_type branch from 9d694ef to 4719773 Compare November 5, 2017 16:49

hypnoce force-pushed the multi_reference_with_reference_type branch from 4719773 to c1adb9c Compare November 5, 2017 21:55

pavolloffay mentioned this pull request Nov 6, 2017

Test reference type in tracing interceptor opentracing-contrib/java-jaxrs#48

Open

pavolloffay reviewed Nov 6, 2017

View reviewed changes

Fixes opentracing#210 : add support for multi reference and expose re…

2ff3277

…ferences context and type from MockSpan.

hypnoce force-pushed the multi_reference_with_reference_type branch from c1adb9c to 2ff3277 Compare November 6, 2017 21:39

pavolloffay approved these changes Nov 7, 2017

View reviewed changes

yurishkuro merged commit dd75c9d into opentracing:master Nov 7, 2017

Conversation

hypnoce commented Nov 5, 2017

Uh oh!

yurishkuro Nov 5, 2017

Choose a reason for hiding this comment

Uh oh!

hypnoce Nov 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yurishkuro Nov 5, 2017

Choose a reason for hiding this comment

Uh oh!

coveralls commented Nov 5, 2017

Uh oh!

yurishkuro Nov 5, 2017

Choose a reason for hiding this comment

Uh oh!

hypnoce Nov 5, 2017

Choose a reason for hiding this comment

Uh oh!

yurishkuro Nov 5, 2017

Choose a reason for hiding this comment

Uh oh!

coveralls commented Nov 5, 2017

Uh oh!

coveralls commented Nov 5, 2017

Uh oh!

yurishkuro commented Nov 6, 2017

Uh oh!

pavolloffay commented Nov 6, 2017

Uh oh!

pavolloffay Nov 6, 2017

Choose a reason for hiding this comment

Uh oh!

hypnoce Nov 6, 2017

Choose a reason for hiding this comment

Uh oh!

hypnoce commented Nov 6, 2017

Uh oh!

coveralls commented Nov 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hypnoce Nov 5, 2017 •

edited

Loading