Skip to content

Commit

Permalink
Address review comment
Browse files Browse the repository at this point in the history
Signed-off-by: Gagan Juneja <gjjuneja@amazon.com>
  • Loading branch information
Gagan Juneja committed Sep 23, 2023
1 parent 83c088f commit 54286ac
Show file tree
Hide file tree
Showing 17 changed files with 239 additions and 259 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
package org.opensearch.telemetry.tracing;

import org.opensearch.common.annotation.InternalApi;
import org.opensearch.telemetry.tracing.attributes.Attributes;

import java.io.Closeable;
import java.io.IOException;
Expand Down Expand Up @@ -44,27 +43,13 @@ public DefaultTracer(TracingTelemetry tracingTelemetry, TracerContextStorage<Str

@Override
public Span startSpan(SpanCreationContext context) {
return startSpan(context.getSpanName(), context.getAttributes(), context.getSpanKind());
}

@Override
public Span startSpan(String spanName, SpanKind spanKind) {
return startSpan(spanName, Attributes.EMPTY, spanKind);
}

@Override
public Span startSpan(String spanName, Attributes attributes, SpanKind spanKind) {
return startSpan(spanName, (SpanContext) null, attributes, spanKind);
}

@Override
public Span startSpan(String spanName, SpanContext parentSpan, Attributes attributes, SpanKind spanKind) {
Span span = null;
if (parentSpan != null) {
span = createSpan(spanName, parentSpan.getSpan(), attributes, spanKind);
Span parentSpan = null;
if (context.getParent() != null) {
parentSpan = context.getParent().getSpan();
} else {
span = createSpan(spanName, getCurrentSpanInternal(), attributes, spanKind);
parentSpan = getCurrentSpanInternal();
}
Span span = createSpan(context, parentSpan);
setCurrentSpanInContext(span);
addDefaultAttributes(span);
return span;
Expand All @@ -87,17 +72,7 @@ public SpanContext getCurrentSpan() {

@Override
public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext) {
return startScopedSpan(spanCreationContext, null);
}

@Override
public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext, SpanContext parentSpan) {
Span span = startSpan(
spanCreationContext.getSpanName(),
parentSpan,
spanCreationContext.getAttributes(),
spanCreationContext.getSpanKind()
);
Span span = startSpan(spanCreationContext);
SpanScope spanScope = withSpanInScope(span);
return new DefaultScopedSpan(span, spanScope);
}
Expand All @@ -107,8 +82,8 @@ public SpanScope withSpanInScope(Span span) {
return DefaultSpanScope.create(span, tracerContextStorage).attach();
}

private Span createSpan(String spanName, Span parentSpan, Attributes attributes, SpanKind spanKind) {
return tracingTelemetry.createSpan(spanName, parentSpan, attributes, spanKind);
private Span createSpan(SpanCreationContext spanCreationContext, Span parentSpan) {
return tracingTelemetry.createSpan(spanCreationContext, parentSpan);
}

private void setCurrentSpanInContext(Span span) {
Expand All @@ -126,12 +101,8 @@ protected void addDefaultAttributes(Span span) {
@Override
public Span startSpan(SpanCreationContext spanCreationContext, Map<String, List<String>> headers) {
Optional<Span> propagatedSpan = tracingTelemetry.getContextPropagator().extractFromHeaders(headers);
return startSpan(
spanCreationContext.getSpanName(),
propagatedSpan.map(SpanContext::new).orElse(null),
spanCreationContext.getAttributes(),
spanCreationContext.getSpanKind()
);
spanCreationContext.parent(propagatedSpan.map(SpanContext::new).orElse(null));
return startSpan(spanCreationContext);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

import org.opensearch.common.annotation.ExperimentalApi;

import java.util.Objects;

/**
* Wrapped Span will be exposed to the code outside of tracing package for sharing the {@link Span} without having access to
* its properties.
Expand All @@ -31,4 +33,17 @@ public SpanContext(Span span) {
Span getSpan() {
return span;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof SpanContext)) return false;
SpanContext that = (SpanContext) o;
return Objects.equals(span, that.span);
}

@Override
public int hashCode() {
return Objects.hash(span);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,36 +11,88 @@
import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.telemetry.tracing.attributes.Attributes;

import java.util.Objects;

/**
* Context for span details.
*
* @opensearch.experimental
*/
@ExperimentalApi
public final class SpanCreationContext {
private final String spanName;
private final Attributes attributes;
private final SpanKind spanKind;
private String spanName;
private Attributes attributes;
private SpanKind spanKind = SpanKind.INTERNAL;
private SpanContext parent;

/**
* Factory method to create {@link SpanCreationContext}
* @return spanCreationContext
*/
public static SpanCreationContext create() {
return new SpanCreationContext();
}

/**
* Constructor.
* @param spanName span name.
* @param attributes attributes.
*/
public SpanCreationContext(String spanName, Attributes attributes) {
this(spanName, attributes, SpanKind.INTERNAL);
private SpanCreationContext() {}

/**
* Sets the span type to server.
* @return spanCreationContext
*/
public SpanCreationContext server() {
this.spanKind = SpanKind.SERVER;
return this;
}

/**
* Sets the span type to client.
* @return spanCreationContext
*/
public SpanCreationContext client() {
this.spanKind = SpanKind.CLIENT;
return this;
}

/**
* Sets the span type to internal.
* @return spanCreationContext
*/
public SpanCreationContext internal() {
this.spanKind = SpanKind.INTERNAL;
return this;
}

/**
* Constructor
* Sets the span name.
* @param spanName span name.
* @param attributes attributes.
* @param spanKind span type.
* @return spanCreationContext
*/
public SpanCreationContext(String spanName, Attributes attributes, SpanKind spanKind) {
public SpanCreationContext name(String spanName) {
this.spanName = spanName;
return this;
}

/**
* Sets the span attributes.
* @param attributes attributes.
* @return spanCreationContext
*/
public SpanCreationContext attributes(Attributes attributes) {
this.attributes = attributes;
this.spanKind = spanKind;
return this;
}

/**
* Sets the parent for spann
* @param parent parent
* @return spanCreationContext
*/
public SpanCreationContext parent(SpanContext parent) {
this.parent = parent;
return this;
}

/**
Expand All @@ -66,4 +118,28 @@ public Attributes getAttributes() {
public SpanKind getSpanKind() {
return spanKind;
}

/**
* Returns the parent span
* @return parent.
*/
public SpanContext getParent() {
return parent;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof SpanCreationContext)) return false;
SpanCreationContext that = (SpanCreationContext) o;
return spanName.equals(that.spanName)
&& attributes.equals(that.attributes)
&& spanKind == that.spanKind
&& parent.equals(that.parent);
}

@Override
public int hashCode() {
return Objects.hash(spanName, attributes, spanKind, parent);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
package org.opensearch.telemetry.tracing;

import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.telemetry.tracing.attributes.Attributes;
import org.opensearch.telemetry.tracing.http.HttpTracer;

import java.io.Closeable;
Expand All @@ -32,37 +31,6 @@ public interface Tracer extends HttpTracer, Closeable {
*/
Span startSpan(SpanCreationContext context);

/**
* Starts the {@link Span} with given name
*
* @param spanName span name
* @param spanKind type of span.
* @return span, must be closed.
*/
Span startSpan(String spanName, SpanKind spanKind);

/**
* Starts the {@link Span} with given name and attributes. This is required in cases when some attribute based
* decision needs to be made before starting the span. Very useful in the case of Sampling.
*
* @param spanName span name.
* @param attributes attributes to be added.
* @param spanKind type of span.
* @return span, must be closed.
*/
Span startSpan(String spanName, Attributes attributes, SpanKind spanKind);

/**
* Starts the {@link Span} with the given name, parent and attributes.
*
* @param spanName span name.
* @param parentSpan parent span.
* @param attributes attributes to be added.
* @param spanKind type of span.
* @return span, must be closed.
*/
Span startSpan(String spanName, SpanContext parentSpan, Attributes attributes, SpanKind spanKind);

/**
* Returns the current span.
* @return current wrapped span.
Expand All @@ -77,15 +45,6 @@ public interface Tracer extends HttpTracer, Closeable {
*/
ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext);

/**
* Start the span and scoped it. This must be used for scenarios where {@link SpanScope} and {@link Span} lifecycles
* are same and ends within the same thread where created.
* @param spanCreationContext span creation context
* @param parentSpan parent span.
* @return scope of the span, must be closed with explicit close or with try-with-resource
*/
ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext, SpanContext parentSpan);

/**
* Creates the Span Scope for a current thread. It's mandatory to scope the span just after creation so that it will
* automatically manage the attach /detach to the current thread.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
package org.opensearch.telemetry.tracing;

import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.telemetry.tracing.attributes.Attributes;

import java.io.Closeable;

Expand All @@ -24,13 +23,11 @@ public interface TracingTelemetry extends Closeable {
/**
* Creates span with provided arguments
*
* @param spanName name of the span
* @param parentSpan span's parent span
* @param attributes attributes to be added.
* @param spanKind type of span.
* @param spanCreationContext span creation context.
* @param parentSpan parent span.
* @return span instance
*/
Span createSpan(String spanName, Span parentSpan, Attributes attributes, SpanKind spanKind);
Span createSpan(SpanCreationContext spanCreationContext, Span parentSpan);

/**
* provides tracing context propagator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@
import org.opensearch.telemetry.tracing.Span;
import org.opensearch.telemetry.tracing.SpanContext;
import org.opensearch.telemetry.tracing.SpanCreationContext;
import org.opensearch.telemetry.tracing.SpanKind;
import org.opensearch.telemetry.tracing.SpanScope;
import org.opensearch.telemetry.tracing.Tracer;
import org.opensearch.telemetry.tracing.attributes.Attributes;

import java.util.List;
import java.util.Map;
Expand All @@ -41,21 +39,6 @@ public Span startSpan(SpanCreationContext context) {
return NoopSpan.INSTANCE;
}

@Override
public Span startSpan(String spanName, SpanKind spanKind) {
return NoopSpan.INSTANCE;
}

@Override
public Span startSpan(String spanName, Attributes attributes, SpanKind spanKind) {
return NoopSpan.INSTANCE;
}

@Override
public Span startSpan(String spanName, SpanContext parentSpan, Attributes attributes, SpanKind spanKind) {
return NoopSpan.INSTANCE;
}

@Override
public SpanContext getCurrentSpan() {
return new SpanContext(NoopSpan.INSTANCE);
Expand All @@ -66,11 +49,6 @@ public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext) {
return ScopedSpan.NO_OP;
}

@Override
public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext, SpanContext parentSpan) {
return ScopedSpan.NO_OP;
}

@Override
public SpanScope withSpanInScope(Span span) {
return SpanScope.NO_OP;
Expand Down
Loading

0 comments on commit 54286ac

Please sign in to comment.