Skip to content

Commit

Permalink
Optimize log hot path (open-telemetry#4913)
Browse files Browse the repository at this point in the history 
* ComponentRegistry accepts name, version, schemaUrl instead of InstrumentationScopeInfo

* Fix comment
  • Loading branch information
@jack-berg @dmarkwat
jack-berg authored and dmarkwat committed Dec 30, 2022
1 parent 8cbc711 commit 8ebf0d7
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 109 deletions.
94 changes: 80 additions & 14 deletions sdk/common/src/main/java/io/opentelemetry/sdk/internal/ComponentRegistry.java
View file
Expand Up @@ -5,46 +5,112 @@

package io.opentelemetry.sdk.internal;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.function.Function;
import javax.annotation.Nullable;

/**
* Component (tracer, meter, etc) registry class for all the provider classes (TracerProvider,
* MeterProvider, etc.).
*
* <p>Components are identified by name, version, and schema. Name is required, but version and
* schema are optional. Therefore, we have 4 possible scenarios for component keys:
*
* <ol>
* <li>Only name is provided, represented by {@link #componentByName}
* <li>Name and version are provided, represented by {@link #componentByNameAndVersion}
* <li>Name and schema are provided, represented by {@link #componentByNameAndSchema}
* <li>Name, version and schema are provided, represented by {@link
* #componentByNameVersionAndSchema}
* </ol>
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*
* @param <V> the type of the registered value.
*/
public final class ComponentRegistry<V> {

private final ConcurrentMap<InstrumentationScopeInfo, V> registry = new ConcurrentHashMap<>();
private final Map<String, V> componentByName = new ConcurrentHashMap<>();
private final Map<String, Map<String, V>> componentByNameAndVersion = new ConcurrentHashMap<>();
private final Map<String, Map<String, V>> componentByNameAndSchema = new ConcurrentHashMap<>();
private final Map<String, Map<String, Map<String, V>>> componentByNameVersionAndSchema =
new ConcurrentHashMap<>();

private final Set<V> allComponents = Collections.newSetFromMap(new IdentityHashMap<>());

private final Function<InstrumentationScopeInfo, V> factory;

public ComponentRegistry(Function<InstrumentationScopeInfo, V> factory) {
this.factory = factory;
}

/**
* Returns the registered value associated with this {@link InstrumentationScopeInfo scope} if
* any, otherwise creates a new instance and associates it with the given scope.
* Returns the component associated with the {@code name}, {@code version}, and {@code schemaUrl}.
* {@link Attributes} are not part of component identity. Behavior is undefined when different
* {@link Attributes} are provided where {@code name}, {@code version}, and {@code schemaUrl} are
* identical.
*/
public V get(InstrumentationScopeInfo instrumentationScopeInfo) {
// Optimistic lookup, before creating the new component.
V component = registry.get(instrumentationScopeInfo);
if (component != null) {
return component;
public V get(
String name, @Nullable String version, @Nullable String schemaUrl, Attributes attributes) {
if (version != null && schemaUrl != null) {
Map<String, Map<String, V>> componentByVersionAndSchema =
componentByNameVersionAndSchema.computeIfAbsent(
name, unused -> new ConcurrentHashMap<>());
Map<String, V> componentBySchema =
componentByVersionAndSchema.computeIfAbsent(version, unused -> new ConcurrentHashMap<>());
return componentBySchema.computeIfAbsent(
schemaUrl,
schemaUrl1 ->
buildComponent(
InstrumentationScopeInfo.builder(name)
.setVersion(version)
.setSchemaUrl(schemaUrl1)
.setAttributes(attributes)
.build()));
} else if (version != null) { // schemaUrl == null
Map<String, V> componentByVersion =
componentByNameAndVersion.computeIfAbsent(name, unused -> new ConcurrentHashMap<>());
return componentByVersion.computeIfAbsent(
version,
version1 ->
buildComponent(
InstrumentationScopeInfo.builder(name)
.setVersion(version1)
.setAttributes(attributes)
.build()));
}
if (schemaUrl != null) { // version == null
Map<String, V> componentBySchema =
componentByNameAndSchema.computeIfAbsent(name, unused -> new ConcurrentHashMap<>());
return componentBySchema.computeIfAbsent(
schemaUrl,
schemaUrl1 ->
buildComponent(
InstrumentationScopeInfo.builder(name)
.setSchemaUrl(schemaUrl1)
.setAttributes(attributes)
.build()));
} else { // schemaUrl == null && version == null
return componentByName.computeIfAbsent(
name,
name1 ->
buildComponent(
InstrumentationScopeInfo.builder(name1).setAttributes(attributes).build()));
}
}

V newComponent = factory.apply(instrumentationScopeInfo);
V oldComponent = registry.putIfAbsent(instrumentationScopeInfo, newComponent);
return oldComponent != null ? oldComponent : newComponent;
private V buildComponent(InstrumentationScopeInfo instrumentationScopeInfo) {
V component = factory.apply(instrumentationScopeInfo);
allComponents.add(component);
return component;
}

/**
Expand All @@ -53,6 +119,6 @@ public V get(InstrumentationScopeInfo instrumentationScopeInfo) {
* @return a {@code Collection} view of the registered components.
*/
public Collection<V> getComponents() {
return Collections.unmodifiableCollection(new ArrayList<>(registry.values()));
return Collections.unmodifiableCollection(allComponents);
}
}
93 changes: 22 additions & 71 deletions sdk/common/src/test/java/io/opentelemetry/sdk/internal/ComponentRegistryTest.java
View file
Expand Up @@ -8,7 +8,6 @@
import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
import org.junit.jupiter.api.Test;

class ComponentRegistryTest {
Expand All @@ -22,83 +21,35 @@ class ComponentRegistryTest {

@Test
void get_SameInstance() {
assertThat(registry.get(InstrumentationScopeInfo.builder(NAME).build()))
.isSameAs(registry.get(InstrumentationScopeInfo.builder(NAME).build()));
assertThat(registry.get(InstrumentationScopeInfo.builder(NAME).setVersion(VERSION).build()))
.isSameAs(registry.get(InstrumentationScopeInfo.builder(NAME).setVersion(VERSION).build()));
assertThat(
registry.get(InstrumentationScopeInfo.builder(NAME).setSchemaUrl(SCHEMA_URL).build()))
assertThat(registry.get(NAME, null, null, Attributes.empty()))
.isSameAs(registry.get(NAME, null, null, Attributes.empty()))
.isSameAs(registry.get(NAME, null, null, Attributes.builder().put("k1", "v2").build()));

assertThat(registry.get(NAME, VERSION, null, Attributes.empty()))
.isSameAs(registry.get(NAME, VERSION, null, Attributes.empty()))
.isSameAs(registry.get(NAME, VERSION, null, Attributes.builder().put("k1", "v2").build()));
assertThat(registry.get(NAME, null, SCHEMA_URL, Attributes.empty()))
.isSameAs(registry.get(NAME, null, SCHEMA_URL, Attributes.empty()))
.isSameAs(
registry.get(InstrumentationScopeInfo.builder(NAME).setSchemaUrl(SCHEMA_URL).build()));
assertThat(
registry.get(InstrumentationScopeInfo.builder(NAME).setAttributes(ATTRIBUTES).build()))
registry.get(NAME, null, SCHEMA_URL, Attributes.builder().put("k1", "v2").build()));
assertThat(registry.get(NAME, VERSION, SCHEMA_URL, Attributes.empty()))
.isSameAs(registry.get(NAME, VERSION, SCHEMA_URL, Attributes.empty()))
.isSameAs(
registry.get(InstrumentationScopeInfo.builder(NAME).setAttributes(ATTRIBUTES).build()));
assertThat(
registry.get(
InstrumentationScopeInfo.builder(NAME)
.setVersion(VERSION)
.setSchemaUrl(SCHEMA_URL)
.setAttributes(ATTRIBUTES)
.build()))
.isSameAs(
registry.get(
InstrumentationScopeInfo.builder(NAME)
.setVersion(VERSION)
.setSchemaUrl(SCHEMA_URL)
.setAttributes(ATTRIBUTES)
.build()));
registry.get(NAME, VERSION, SCHEMA_URL, Attributes.builder().put("k1", "v2").build()));
}

@Test
void get_DifferentInstance() {
InstrumentationScopeInfo allFields =
InstrumentationScopeInfo.builder(NAME)
.setVersion(VERSION)
.setSchemaUrl(SCHEMA_URL)
.setAttributes(ATTRIBUTES)
.build();
assertThat(registry.get(NAME, VERSION, SCHEMA_URL, ATTRIBUTES))
.isNotSameAs(registry.get(NAME + "_1", VERSION, SCHEMA_URL, ATTRIBUTES))
.isNotSameAs(registry.get(NAME, VERSION + "_1", SCHEMA_URL, ATTRIBUTES))
.isNotSameAs(registry.get(NAME, VERSION, SCHEMA_URL + "_1", ATTRIBUTES));

assertThat(registry.get(NAME, VERSION, null, Attributes.empty()))
.isNotSameAs(registry.get(NAME, null, null, Attributes.empty()));

assertThat(registry.get(allFields))
.isNotSameAs(
registry.get(
InstrumentationScopeInfo.builder(NAME + "_1")
.setVersion(VERSION)
.setSchemaUrl(SCHEMA_URL)
.setAttributes(ATTRIBUTES)
.build()));
assertThat(registry.get(allFields))
.isNotSameAs(
registry.get(
InstrumentationScopeInfo.builder(NAME)
.setVersion(VERSION + "_1")
.setSchemaUrl(SCHEMA_URL)
.setAttributes(ATTRIBUTES)
.build()));
assertThat(registry.get(allFields))
.isNotSameAs(
registry.get(
InstrumentationScopeInfo.builder(NAME)
.setVersion(VERSION)
.setSchemaUrl(SCHEMA_URL + "_1")
.setAttributes(ATTRIBUTES)
.build()));
assertThat(registry.get(allFields))
.isNotSameAs(
registry.get(
InstrumentationScopeInfo.builder(NAME)
.setVersion(VERSION)
.setSchemaUrl(SCHEMA_URL)
.setAttributes(Attributes.builder().put("k1", "v2").build())
.build()));
assertThat(registry.get(InstrumentationScopeInfo.builder(NAME).setVersion(VERSION).build()))
.isNotSameAs(registry.get(InstrumentationScopeInfo.builder(NAME).build()));
assertThat(
registry.get(InstrumentationScopeInfo.builder(NAME).setSchemaUrl(SCHEMA_URL).build()))
.isNotSameAs(registry.get(InstrumentationScopeInfo.builder(NAME).build()));
assertThat(
registry.get(InstrumentationScopeInfo.builder(NAME).setAttributes(ATTRIBUTES).build()))
.isNotSameAs(registry.get(InstrumentationScopeInfo.builder(NAME).build()));
assertThat(registry.get(NAME, null, SCHEMA_URL, Attributes.empty()))
.isNotSameAs(registry.get(NAME, null, null, Attributes.empty()));
}

private static final class TestComponent {}
Expand Down
17 changes: 10 additions & 7 deletions sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLoggerBuilder.java
View file
Expand Up @@ -5,21 +5,22 @@

package io.opentelemetry.sdk.logs;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.logs.LoggerBuilder;
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
import io.opentelemetry.sdk.common.InstrumentationScopeInfoBuilder;
import io.opentelemetry.sdk.internal.ComponentRegistry;
import javax.annotation.Nullable;

final class SdkLoggerBuilder implements LoggerBuilder {

private final ComponentRegistry<SdkLogger> registry;
private final InstrumentationScopeInfoBuilder scopeBuilder;
private final String instrumentationScopeName;
@Nullable private String instrumentationScopeVersion;
@Nullable private String schemaUrl;
@Nullable private String eventDomain;

SdkLoggerBuilder(ComponentRegistry<SdkLogger> registry, String instrumentationScopeName) {
this.registry = registry;
this.scopeBuilder = InstrumentationScopeInfo.builder(instrumentationScopeName);
this.instrumentationScopeName = instrumentationScopeName;
}

@Override
Expand All @@ -30,19 +31,21 @@ public LoggerBuilder setEventDomain(String eventDomain) {

@Override
public SdkLoggerBuilder setSchemaUrl(String schemaUrl) {
scopeBuilder.setSchemaUrl(schemaUrl);
this.schemaUrl = schemaUrl;
return this;
}

@Override
public SdkLoggerBuilder setInstrumentationVersion(String instrumentationScopeVersion) {
scopeBuilder.setVersion(instrumentationScopeVersion);
this.instrumentationScopeVersion = instrumentationScopeVersion;
return this;
}

@Override
public SdkLogger build() {
SdkLogger logger = registry.get(scopeBuilder.build());
SdkLogger logger =
registry.get(
instrumentationScopeName, instrumentationScopeVersion, schemaUrl, Attributes.empty());
return eventDomain == null ? logger : logger.withEventDomain(eventDomain);
}
}
14 changes: 11 additions & 3 deletions sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLoggerProvider.java
View file
Expand Up @@ -5,6 +5,7 @@

package io.opentelemetry.sdk.logs;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.logs.Logger;
import io.opentelemetry.api.logs.LoggerBuilder;
import io.opentelemetry.api.logs.LoggerProvider;
Expand All @@ -17,6 +18,7 @@
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
import java.util.logging.Level;
import javax.annotation.Nullable;

/** SDK implementation for {@link LoggerProvider}. */
public final class SdkLoggerProvider implements LoggerProvider, Closeable {
Expand Down Expand Up @@ -61,7 +63,8 @@ public static SdkLoggerProviderBuilder builder() {
*/
@Override
public Logger get(String instrumentationScopeName) {
return loggerBuilder(instrumentationScopeName).build();
return loggerComponentRegistry.get(
instrumentationNameOrDefault(instrumentationScopeName), null, null, Attributes.empty());
}

/**
Expand All @@ -75,11 +78,16 @@ public LoggerBuilder loggerBuilder(String instrumentationScopeName) {
if (isNoopLogRecordProcessor) {
return LoggerProvider.noop().loggerBuilder(instrumentationScopeName);
}
return new SdkLoggerBuilder(
loggerComponentRegistry, instrumentationNameOrDefault(instrumentationScopeName));
}

private static String instrumentationNameOrDefault(@Nullable String instrumentationScopeName) {
if (instrumentationScopeName == null || instrumentationScopeName.isEmpty()) {
LOGGER.fine("Logger requested without instrumentation scope name.");
instrumentationScopeName = DEFAULT_LOGGER_NAME;
return DEFAULT_LOGGER_NAME;
}
return new SdkLoggerBuilder(loggerComponentRegistry, instrumentationScopeName);
return instrumentationScopeName;
}

/**
Expand Down
17 changes: 10 additions & 7 deletions sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeterBuilder.java
View file
Expand Up @@ -5,36 +5,39 @@

package io.opentelemetry.sdk.metrics;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.metrics.Meter;
import io.opentelemetry.api.metrics.MeterBuilder;
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
import io.opentelemetry.sdk.common.InstrumentationScopeInfoBuilder;
import io.opentelemetry.sdk.internal.ComponentRegistry;
import javax.annotation.Nullable;

class SdkMeterBuilder implements MeterBuilder {

private final ComponentRegistry<SdkMeter> registry;
private final InstrumentationScopeInfoBuilder scopeBuilder;
private final String instrumentationScopeName;
@Nullable private String instrumentationScopeVersion;
@Nullable private String schemaUrl;

SdkMeterBuilder(ComponentRegistry<SdkMeter> registry, String instrumentationScopeName) {
this.registry = registry;
this.scopeBuilder = InstrumentationScopeInfo.builder(instrumentationScopeName);
this.instrumentationScopeName = instrumentationScopeName;
}

@Override
public MeterBuilder setSchemaUrl(String schemaUrl) {
scopeBuilder.setSchemaUrl(schemaUrl);
this.schemaUrl = schemaUrl;
return this;
}

@Override
public MeterBuilder setInstrumentationVersion(String instrumentationScopeVersion) {
scopeBuilder.setVersion(instrumentationScopeVersion);
this.instrumentationScopeVersion = instrumentationScopeVersion;
return this;
}

@Override
public Meter build() {
return registry.get(scopeBuilder.build());
return registry.get(
instrumentationScopeName, instrumentationScopeVersion, schemaUrl, Attributes.empty());
}
}

0 comments on commit 8ebf0d7

Please sign in to comment.