Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove MessageEvent and make Event extensible. #81

Merged
merged 3 commits into from
Apr 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions api/src/main/java/openconsensus/trace/BlankSpan.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import java.util.Map;
import javax.annotation.concurrent.Immutable;
import openconsensus.internal.Utils;
import openconsensus.trace.data.Event;
import openconsensus.trace.data.MessageEvent;

/**
* The {@code BlankSpan} is a singleton class, which is the default {@link Span} that is used when
Expand Down Expand Up @@ -82,11 +80,6 @@ public void addEvent(Event event) {
Utils.checkNotNull(event, "event");
}

@Override
public void addMessageEvent(MessageEvent messageEvent) {
Utils.checkNotNull(messageEvent, "messageEvent");
}

@Override
public void addLink(Link link) {
Utils.checkNotNull(link, "link");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,44 @@
* limitations under the License.
*/

package openconsensus.trace.data;
package openconsensus.trace;

import com.google.auto.value.AutoValue;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.concurrent.Immutable;
import openconsensus.internal.Utils;
import openconsensus.trace.AttributeValue;

/**
* A text annotation with a set of attributes.
*
* @since 0.1.0
*/
@Immutable
@AutoValue
public abstract class Event {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API should allow to create an Event. We don't have this option any longer, do we?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User can add events via the Span.event api which supports all the variants. Do we still need an event implementation here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you create event for spandata?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment. Any reason to create public type, not simply return it from the static method on Event?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Let's address the interface change separately.

private static final Map<String, AttributeValue> EMPTY_ATTRIBUTES =
Collections.unmodifiableMap(Collections.<String, AttributeValue>emptyMap());

/**
* Return the name of the {@code Event}.
*
* @return the name of the {@code Event}.
* @since 0.1.0
*/
public abstract String getName();

/**
* Return the attributes of the {@code Event}.
*
* @return the attributes of the {@code Event}.
* @since 0.1.0
*/
public abstract Map<String, AttributeValue> getAttributes();

/** Protected constructor to allow subclassing this class. */
protected Event() {}

/**
* Returns a new {@code Event} with the given name.
*
Expand All @@ -43,8 +60,8 @@ public abstract class Event {
* @throws NullPointerException if {@code name} is {@code null}.
* @since 0.1.0
*/
public static Event fromName(String name) {
return new AutoValue_Event(name, EMPTY_ATTRIBUTES);
public static Event create(String name) {
return new AutoValue_Event_ImmutableEvent(name, EMPTY_ATTRIBUTES);
}

/**
Expand All @@ -56,28 +73,20 @@ public static Event fromName(String name) {
* @throws NullPointerException if {@code name} or {@code attributes} are {@code null}.
* @since 0.1.0
*/
public static Event fromNameAndAttributes(String name, Map<String, AttributeValue> attributes) {
return new AutoValue_Event(
public static Event create(String name, Map<String, AttributeValue> attributes) {
return new AutoValue_Event_ImmutableEvent(
name,
Collections.unmodifiableMap(
new HashMap<String, AttributeValue>(Utils.checkNotNull(attributes, "attributes"))));
Collections.unmodifiableMap(new HashMap<>(Utils.checkNotNull(attributes, "attributes"))));
}

/**
* Return the name of the {@code Event}.
* A text annotation with a set of attributes.
*
* @return the name of the {@code Event}.
* @since 0.1.0
*/
public abstract String getName();

/**
* Return the attributes of the {@code Event}.
*
* @return the attributes of the {@code Event}.
* @since 0.1.0
*/
public abstract Map<String, AttributeValue> getAttributes();

Event() {}
@Immutable
@AutoValue
abstract static class ImmutableEvent extends Event {
ImmutableEvent() {}
}
}
15 changes: 0 additions & 15 deletions api/src/main/java/openconsensus/trace/Span.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
package openconsensus.trace;

import java.util.Map;
import openconsensus.trace.data.Event;
import openconsensus.trace.data.MessageEvent;

/**
* An abstract class that represents a span. It has an associated {@link SpanContext}.
Expand Down Expand Up @@ -106,19 +104,6 @@ public abstract class Span {
*/
public abstract void addEvent(Event event);

/**
* Adds a MessageEvent to the {@code Span}.
*
* <p>This function can be used by higher level applications to record messaging event.
*
* <p>This method should always be overridden by users whose API versions are larger or equal to
* {@code 0.12}.
*
* @param messageEvent the message to add.
* @since 0.1.0
*/
public abstract void addMessageEvent(MessageEvent messageEvent);

/**
* Adds a {@link Link} to the {@code Span}.
*
Expand Down
150 changes: 0 additions & 150 deletions api/src/main/java/openconsensus/trace/data/MessageEvent.java

This file was deleted.

15 changes: 2 additions & 13 deletions api/src/main/java/openconsensus/trace/data/SpanData.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import openconsensus.common.Timestamp;
import openconsensus.internal.Utils;
import openconsensus.trace.AttributeValue;
import openconsensus.trace.Event;
import openconsensus.trace.Link;
import openconsensus.trace.Span;
import openconsensus.trace.Span.Kind;
Expand Down Expand Up @@ -54,8 +55,6 @@ public abstract class SpanData {
* @param startTimestamp the start {@code Timestamp} of the {@code Span}.
* @param attributes the attributes associated with the {@code Span}.
* @param events the events associated with the {@code Span}.
* @param messageEvents the message events (or network events for backward compatibility)
* associated with the {@code Span}.
* @param links the links associated with the {@code Span}.
* @param childSpanCount the number of child spans that were generated while the span was active.
* @param status the {@code Status} of the {@code Span}. {@code null} if the {@code Span} is still
Expand All @@ -73,12 +72,11 @@ public static SpanData create(
Timestamp startTimestamp,
Attributes attributes,
TimedEvents<Event> events,
TimedEvents<MessageEvent> messageEvents,
Links links,
@Nullable Integer childSpanCount,
@Nullable Status status,
@Nullable Timestamp endTimestamp) {
Utils.checkNotNull(messageEvents, "messageEvents");
Utils.checkNotNull(events, "events");
return new AutoValue_SpanData(
context,
parentSpanId,
Expand All @@ -87,7 +85,6 @@ public static SpanData create(
startTimestamp,
attributes,
events,
messageEvents,
links,
childSpanCount,
status,
Expand Down Expand Up @@ -151,14 +148,6 @@ public static SpanData create(
*/
public abstract TimedEvents<Event> getEvents();

/**
* Returns message events recorded for this {@code Span}.
*
* @return message events recorded for this {@code Span}.
* @since 0.1.0
*/
public abstract TimedEvents<MessageEvent> getMessageEvents();

/**
* Returns links recorded for this {@code Span}.
*
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ subprojects {
apply plugin: 'com.github.sherter.google-java-format'
apply plugin: "net.ltgt.errorprone"

group = "io.openconsensus"
group = "openconsensus"
version = "0.1.0-SNAPSHOT" // CURRENT_VERSION

sourceCompatibility = 1.7
Expand Down
6 changes: 6 additions & 0 deletions contrib/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
OpenCensus API
======================================================

* Java 7 and Android 14 compatible.
* The abstract classes in this directory can be subclassed to create alternative
implementations of the OpenCensus library.
15 changes: 15 additions & 0 deletions contrib/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
description = 'OpenConsensus Contrib'

dependencies {
compile project(':openconsensus-api')

compileOnly libraries.auto_value

signature "org.codehaus.mojo.signature:java17:1.0@signature"
signature "net.sf.androidscents.signature:android-api-level-14:4.0_r4@signature"
}

javadoc {
exclude 'io/opencensus/internal/**'
exclude 'io/opencensus/trace/internal/**'
}
Loading