Skip to content

Commit

Permalink
Fix comments from code review.
Browse files Browse the repository at this point in the history
  • Loading branch information
jsuereth committed Nov 27, 2024
1 parent 0ed5fe2 commit 8b4f48f
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 47 deletions.
63 changes: 62 additions & 1 deletion docs/apidiffs/current_vs_latest/opentelemetry-sdk-common.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,63 @@
Comparing source compatibility of opentelemetry-sdk-common-1.45.0-SNAPSHOT.jar against opentelemetry-sdk-common-1.44.1.jar
No changes.
+++ NEW CLASS: PUBLIC(+) FINAL(+) io.opentelemetry.sdk.resources.detectors.ServiceDetector (not serializable)
+++ CLASS FILE FORMAT VERSION: 52.0 <- n.a.
+++ NEW INTERFACE: io.opentelemetry.sdk.resources.EntityDetector
+++ NEW SUPERCLASS: java.lang.Object
+++ NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) io.opentelemetry.sdk.resources.EntityDetector INSTANCE
+++ NEW METHOD: PUBLIC(+) java.util.List<io.opentelemetry.sdk.resources.Entity> detectEntities()
+++ NEW CLASS: PUBLIC(+) FINAL(+) io.opentelemetry.sdk.resources.detectors.ServiceInstanceDetector (not serializable)
+++ CLASS FILE FORMAT VERSION: 52.0 <- n.a.
+++ NEW INTERFACE: io.opentelemetry.sdk.resources.EntityDetector
+++ NEW SUPERCLASS: java.lang.Object
+++ NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) io.opentelemetry.sdk.resources.EntityDetector INSTANCE
+++ NEW METHOD: PUBLIC(+) java.util.List<io.opentelemetry.sdk.resources.Entity> detectEntities()
+++ NEW CLASS: PUBLIC(+) FINAL(+) io.opentelemetry.sdk.resources.detectors.TelemetrySdkDetector (not serializable)
+++ CLASS FILE FORMAT VERSION: 52.0 <- n.a.
+++ NEW INTERFACE: io.opentelemetry.sdk.resources.EntityDetector
+++ NEW SUPERCLASS: java.lang.Object
+++ NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) io.opentelemetry.sdk.resources.detectors.TelemetrySdkDetector INSTANCE
+++ NEW METHOD: PUBLIC(+) java.util.List<io.opentelemetry.sdk.resources.Entity> detectEntities()
+++ NEW CLASS: PUBLIC(+) ABSTRACT(+) io.opentelemetry.sdk.resources.Entity (not serializable)
+++ CLASS FILE FORMAT VERSION: 52.0 <- n.a.
+++ NEW SUPERCLASS: java.lang.Object
+++ NEW CONSTRUCTOR: PUBLIC(+) Entity()
+++ NEW METHOD: PUBLIC(+) STATIC(+) FINAL(+) io.opentelemetry.sdk.resources.EntityBuilder builder(java.lang.String)
+++ NEW METHOD: PUBLIC(+) ABSTRACT(+) io.opentelemetry.api.common.Attributes getAttributes()
+++ NEW METHOD: PUBLIC(+) ABSTRACT(+) io.opentelemetry.api.common.Attributes getIdentifyingAttributes()
+++ NEW METHOD: PUBLIC(+) ABSTRACT(+) java.lang.String getSchemaUrl()
+++ NEW ANNOTATION: javax.annotation.Nullable
+++ NEW METHOD: PUBLIC(+) ABSTRACT(+) java.lang.String getType()
+++ NEW METHOD: PUBLIC(+) FINAL(+) io.opentelemetry.sdk.resources.EntityBuilder toBuilder()
+++ NEW CLASS: PUBLIC(+) io.opentelemetry.sdk.resources.EntityBuilder (not serializable)
+++ CLASS FILE FORMAT VERSION: 52.0 <- n.a.
+++ NEW SUPERCLASS: java.lang.Object
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.resources.Entity build()
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.resources.EntityBuilder setSchemaUrl(java.lang.String)
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.resources.EntityBuilder withDescriptive(java.util.function.Consumer<io.opentelemetry.api.common.AttributesBuilder>)
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.resources.EntityBuilder withIdentifying(java.util.function.Consumer<io.opentelemetry.api.common.AttributesBuilder>)
+++ NEW INTERFACE: PUBLIC(+) ABSTRACT(+) io.opentelemetry.sdk.resources.EntityDetector (not serializable)
+++ CLASS FILE FORMAT VERSION: 52.0 <- n.a.
+++ NEW SUPERCLASS: java.lang.Object
+++ NEW METHOD: PUBLIC(+) ABSTRACT(+) java.util.List<io.opentelemetry.sdk.resources.Entity> detectEntities()
**** MODIFIED CLASS: PUBLIC ABSTRACT io.opentelemetry.sdk.resources.Resource (not serializable)
=== CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.resources.Resource create(io.opentelemetry.api.common.Attributes, java.lang.String, java.util.Collection<io.opentelemetry.sdk.resources.Entity>)
*** MODIFIED METHOD: PUBLIC NON_ABSTRACT (<- ABSTRACT) io.opentelemetry.api.common.Attributes getAttributes()
+++* NEW METHOD: PUBLIC(+) ABSTRACT(+) java.util.Collection<io.opentelemetry.sdk.resources.Entity> getEntities()
*** MODIFIED CLASS: PUBLIC io.opentelemetry.sdk.resources.ResourceBuilder (not serializable)
=== CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.resources.ResourceBuilder add(io.opentelemetry.sdk.resources.Entity)
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.resources.ResourceBuilder addAll(java.util.Collection<io.opentelemetry.sdk.resources.Entity>)
+++ NEW CLASS: PUBLIC(+) FINAL(+) io.opentelemetry.sdk.resources.ResourceProvider (not serializable)
+++ CLASS FILE FORMAT VERSION: 52.0 <- n.a.
+++ NEW SUPERCLASS: java.lang.Object
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.resources.ResourceProviderBuilder builder()
+++ NEW METHOD: PUBLIC(+) FINAL(+) io.opentelemetry.sdk.resources.Resource getResource()
+++ NEW CLASS: PUBLIC(+) FINAL(+) io.opentelemetry.sdk.resources.ResourceProviderBuilder (not serializable)
+++ CLASS FILE FORMAT VERSION: 52.0 <- n.a.
+++ NEW SUPERCLASS: java.lang.Object
+++ NEW CONSTRUCTOR: PUBLIC(+) ResourceProviderBuilder()
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.resources.ResourceProviderBuilder addDetectedResource(io.opentelemetry.sdk.resources.Resource)
+++ NEW ANNOTATION: java.lang.Deprecated
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.resources.ResourceProviderBuilder addEntityDetector(io.opentelemetry.sdk.resources.EntityDetector)
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.resources.ResourceProvider build()
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ void toJsonResourceWithEntity() throws Exception {
Resource resource =
Resource.builder()
.add(
Entity.builder()
Entity.builder("test")
.setSchemaUrl("http://example.com/1.0")
.setEntityType("test")
.withIdentifying(attr -> attr.put("test.id", 1))
.withDescriptive(attr -> attr.put("test.name", "one"))
.build())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ public abstract class Entity {
public abstract Attributes getAttributes();

/**
* Returns the URL of the OpenTelemetry schema used by this resource. May be null.
* Returns the URL of the OpenTelemetry schema used by this resource. May be null if this entity
* does not abide by schema conventions (i.e. is custom).
*
* @return An OpenTelemetry schema URL.
* @since 1.4.0
Expand All @@ -68,7 +69,7 @@ public final EntityBuilder toBuilder() {
return new EntityBuilder(this);
}

public static final EntityBuilder builder() {
return new EntityBuilder();
public static final EntityBuilder builder(String entityType) {
return new EntityBuilder(entityType);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
import java.util.Objects;
import java.util.function.Consumer;
import javax.annotation.Nullable;

Expand All @@ -16,12 +15,13 @@
* well as type and schema_url.
*/
public class EntityBuilder {
@Nullable private String entityType;
private final String entityType;
private final AttributesBuilder attributesBuilder;
private final AttributesBuilder identifyingBuilder;
@Nullable private String schemaUrl;

EntityBuilder() {
EntityBuilder(String entityType) {
this.entityType = entityType;
this.attributesBuilder = Attributes.builder();
this.identifyingBuilder = Attributes.builder();
}
Expand All @@ -44,17 +44,6 @@ public EntityBuilder setSchemaUrl(String schemaUrl) {
return this;
}

/**
* Assign an Entity type to the resulting Entity.
*
* @param entityType The type name of the entity.
* @return this
*/
public EntityBuilder setEntityType(String entityType) {
this.entityType = entityType;
return this;
}

/**
* Modify the descriptive attributes of this Entity.
*
Expand All @@ -80,7 +69,6 @@ public EntityBuilder withIdentifying(Consumer<AttributesBuilder> f) {
/** Create the {@link Entity} from this. */
public Entity build() {
// TODO - Better Checks.
Objects.requireNonNull(this.entityType);
return Entity.create(
entityType, identifyingBuilder.build(), attributesBuilder.build(), schemaUrl);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ private static String getServiceName() {
@Override
public List<Entity> detectEntities() {
return Collections.singletonList(
Entity.builder()
.setEntityType(ENTITY_TYPE)
Entity.builder(ENTITY_TYPE)
.setSchemaUrl(SCHEMA_URL)
.withIdentifying(
builder -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ private static String getInstanceId() {
@Override
public List<Entity> detectEntities() {
return Collections.singletonList(
Entity.builder()
.setEntityType(ENTITY_TYPE)
Entity.builder(ENTITY_TYPE)
.setSchemaUrl(SCHEMA_URL)
.withIdentifying(

Check warning on line 33 in sdk/common/src/main/java/io/opentelemetry/sdk/resources/detectors/ServiceInstanceDetector.java

View check run for this annotation

Codecov / codecov/patch

sdk/common/src/main/java/io/opentelemetry/sdk/resources/detectors/ServiceInstanceDetector.java#L30-L33

Added lines #L30 - L33 were not covered by tests
builder -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ public final class TelemetrySdkDetector implements EntityDetector {

static {
TELEMETRY_SDK =
Entity.builder()
.setEntityType(ENTITY_TYPE)
Entity.builder(ENTITY_TYPE)
.setSchemaUrl(SCHEMA_URL)
.withIdentifying(
attributes -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,7 @@ void testMergeResources_entities_separate_types() {
Resource resource1 =
Resource.builder()
.add(
Entity.builder()
.setEntityType("a")
Entity.builder("a")
.setSchemaUrl("one")
.withIdentifying(
builder -> {
Expand All @@ -253,8 +252,7 @@ void testMergeResources_entities_separate_types() {
Resource resource2 =
Resource.builder()
.add(
Entity.builder()
.setEntityType("b")
Entity.builder("b")
.setSchemaUrl("one")
.withIdentifying(
builder -> {
Expand All @@ -274,8 +272,7 @@ void testMergeResources_entities_separate_types_and_schema() {
Resource resource1 =
Resource.builder()
.add(
Entity.builder()
.setEntityType("a")
Entity.builder("a")
.setSchemaUrl("one")
.withIdentifying(
builder -> {
Expand All @@ -286,8 +283,7 @@ void testMergeResources_entities_separate_types_and_schema() {
Resource resource2 =
Resource.builder()
.add(
Entity.builder()
.setEntityType("b")
Entity.builder("b")
.setSchemaUrl("two")
.withIdentifying(
builder -> {
Expand All @@ -307,8 +303,7 @@ void testMergeResources_entities_same_types_and_id() {
Resource resource1 =
Resource.builder()
.add(
Entity.builder()
.setEntityType("a")
Entity.builder("a")
.setSchemaUrl("one")
.withIdentifying(
builder -> {
Expand All @@ -323,8 +318,7 @@ void testMergeResources_entities_same_types_and_id() {
Resource resource2 =
Resource.builder()
.add(
Entity.builder()
.setEntityType("a")
Entity.builder("a")
.setSchemaUrl("one")
.withIdentifying(
builder -> {
Expand All @@ -349,8 +343,7 @@ void testMergeResources_entity_vs_raw_attribute() {
Resource resource1 =
Resource.builder()
.add(
Entity.builder()
.setEntityType("a")
Entity.builder("a")
.setSchemaUrl("one")
.withIdentifying(
builder -> {
Expand All @@ -376,8 +369,7 @@ void testMergeResources_entities_same_types_different_id() {
Resource resource1 =
Resource.builder()
.add(
Entity.builder()
.setEntityType("a")
Entity.builder("a")
.setSchemaUrl("one")
.withIdentifying(
builder -> {
Expand All @@ -392,8 +384,7 @@ void testMergeResources_entities_same_types_different_id() {
Resource resource2 =
Resource.builder()
.add(
Entity.builder()
.setEntityType("a")
Entity.builder("a")
.setSchemaUrl("one")
.withIdentifying(
builder -> {
Expand Down Expand Up @@ -514,9 +505,8 @@ public void build_withEntities() {
Resource resource =
Resource.builder()
.add(
Entity.builder()
Entity.builder("a")
.setSchemaUrl("http://example.com")
.setEntityType("a")
.withIdentifying(id -> id.put("a", "b"))
.build())
.build();
Expand Down

0 comments on commit 8b4f48f

Please sign in to comment.