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

Add autoconfigure console alias for logging exporter #6027

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.exporter.logging.internal;

import io.opentelemetry.exporter.logging.SystemOutLogRecordExporter;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.logs.ConfigurableLogRecordExporterProvider;
import io.opentelemetry.sdk.logs.export.LogRecordExporter;

/**
* {@link LogRecordExporter} SPI implementation for {@link SystemOutLogRecordExporter}.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class ConsoleLogRecordExporterProvider
implements ConfigurableLogRecordExporterProvider {
@Override
public LogRecordExporter createExporter(ConfigProperties config) {
return SystemOutLogRecordExporter.create();
}

@Override
public String getName() {
return "console";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.exporter.logging.internal;

import io.opentelemetry.exporter.logging.LoggingMetricExporter;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.metrics.ConfigurableMetricExporterProvider;
import io.opentelemetry.sdk.metrics.export.MetricExporter;

/**
* {@link MetricExporter} SPI implementation for {@link LoggingMetricExporter}.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class ConsoleMetricExporterProvider implements ConfigurableMetricExporterProvider {
@Override
public MetricExporter createExporter(ConfigProperties config) {
return LoggingMetricExporter.create();

Check warning on line 22 in exporters/logging/src/main/java/io/opentelemetry/exporter/logging/internal/ConsoleMetricExporterProvider.java

View check run for this annotation

Codecov / codecov/patch

exporters/logging/src/main/java/io/opentelemetry/exporter/logging/internal/ConsoleMetricExporterProvider.java#L22

Added line #L22 was not covered by tests
}

@Override
public String getName() {
return "console";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.exporter.logging.internal;

import io.opentelemetry.exporter.logging.LoggingSpanExporter;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.traces.ConfigurableSpanExporterProvider;
import io.opentelemetry.sdk.trace.export.SpanExporter;

/**
* {@link SpanExporter} SPI implementation for {@link LoggingSpanExporter}.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class ConsoleSpanExporterProvider implements ConfigurableSpanExporterProvider {
@Override
public SpanExporter createExporter(ConfigProperties config) {
return LoggingSpanExporter.create();
}

@Override
public String getName() {
return "console";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*
* @deprecated The name {@code logging} is a deprecated alias for {@code console}, which is provided
* via {@link ConsoleLogRecordExporterProvider}.
*/
public class LoggingLogRecordExporterProvider implements ConfigurableLogRecordExporterProvider {
@Deprecated
public final class LoggingLogRecordExporterProvider
implements ConfigurableLogRecordExporterProvider {
@Override
public LogRecordExporter createExporter(ConfigProperties config) {
return SystemOutLogRecordExporter.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*
* @deprecated The name {@code logging} is a deprecated alias for {@code console}, which is provided
* via {@link ConsoleMetricExporterProvider}.
*/
public class LoggingMetricExporterProvider implements ConfigurableMetricExporterProvider {
@Deprecated
public final class LoggingMetricExporterProvider implements ConfigurableMetricExporterProvider {
@Override
public MetricExporter createExporter(ConfigProperties config) {
return LoggingMetricExporter.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*
* @deprecated The name {@code logging} is a deprecated alias for {@code console}, which is provided
* via {@link ConsoleSpanExporterProvider}.
*/
public class LoggingSpanExporterProvider implements ConfigurableSpanExporterProvider {
@Deprecated
public final class LoggingSpanExporterProvider implements ConfigurableSpanExporterProvider {
@Override
public SpanExporter createExporter(ConfigProperties config) {
return LoggingSpanExporter.create();
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
io.opentelemetry.exporter.logging.internal.LoggingLogRecordExporterProvider
io.opentelemetry.exporter.logging.internal.ConsoleLogRecordExporterProvider
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
io.opentelemetry.exporter.logging.internal.LoggingMetricExporterProvider
io.opentelemetry.exporter.logging.internal.ConsoleMetricExporterProvider
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
io.opentelemetry.exporter.logging.internal.LoggingSpanExporterProvider
io.opentelemetry.exporter.logging.internal.ConsoleSpanExporterProvider
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ final class LogRecordExporterConfiguration {

static {
EXPORTER_ARTIFACT_ID_BY_NAME = new HashMap<>();
EXPORTER_ARTIFACT_ID_BY_NAME.put("console", "opentelemetry-exporter-logging");
Copy link
Member

Choose a reason for hiding this comment

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

what do you think of renaming the artifact?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be a little hesitant in renaming artifacts. It's definitely caused a lot of confusion in the past when suddenly an artifact no longer exists for a version, and it's been renamed. This has often led to people mixing and matching artifact versions, which is a significant foot gun for users.

Copy link
Member Author

Choose a reason for hiding this comment

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

An argument against renaming artifacts is that it will cause the class names to be misaligned:

  • Artifact named opentelemetry-exporter-console
  • Classes named Logging{Signal}Exporter

Of course we could introduce new classes named Console{Signal}Exporter and deprecate the Logging{Signal}Exporter variations.

Copy link
Member

Choose a reason for hiding this comment

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

I find the name opentelemetry-exporter-logging to be regularly confusing (I keep thinking it's the log exporter).

I'd be a little hesitant in renaming artifacts. It's definitely caused a lot of confusion in the past when suddenly an artifact no longer exists for a version, and it's been renamed. This has often led to people mixing and matching artifact versions, which is a significant foot gun for users.

this is a great point though

maybe we can have both opentelemetry-exporter-console and keep publishing (a deprecated) opentelemetry-exporter-logging artifact (which can have a dependency on opentelemetry-exporter-console if that's helpful for reducing duplication)?

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe we can have both opentelemetry-exporter-console and keep publishing (a deprecated) opentelemetry-exporter-logging artifact (which can have a dependency on opentelemetry-exporter-console if that's helpful for reducing duplication)?

That's an interesting idea..

While we're on the subject of making the logging exporters less confusing / more useful, would probably be good to optionally give the user more control over where the logs go. Could have an optional Consumer<String> logHandler argument which defaults to sending to JUL, but could also be configured to go to the user's desired logger (SLF4J, System.out, etc). Could apply the same principle to the logging OTLP exporter.

Copy link
Member Author

@jack-berg jack-berg Feb 29, 2024

Choose a reason for hiding this comment

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

Opened #6263 to discuss further and track. Shouldn't be a blocker for this PR tho. Please take another look!

EXPORTER_ARTIFACT_ID_BY_NAME.put("logging", "opentelemetry-exporter-logging");
EXPORTER_ARTIFACT_ID_BY_NAME.put("logging-otlp", "opentelemetry-exporter-logging-otlp");
EXPORTER_ARTIFACT_ID_BY_NAME.put("otlp", "opentelemetry-exporter-otlp");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@
import java.io.Closeable;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.BiFunction;

final class LoggerProviderConfiguration {

private static final List<String> simpleProcessorExporterNames =
Arrays.asList("console", "logging");

static void configureLoggerProvider(
SdkLoggerProviderBuilder loggerProviderBuilder,
ConfigProperties config,
Expand Down Expand Up @@ -55,11 +59,13 @@ static List<LogRecordProcessor> configureLogRecordProcessors(
Map<String, LogRecordExporter> exportersByNameCopy = new HashMap<>(exportersByName);
List<LogRecordProcessor> logRecordProcessors = new ArrayList<>();

LogRecordExporter exporter = exportersByNameCopy.remove("logging");
if (exporter != null) {
LogRecordProcessor logRecordProcessor = SimpleLogRecordProcessor.create(exporter);
closeables.add(logRecordProcessor);
logRecordProcessors.add(logRecordProcessor);
for (String simpleProcessorExporterName : simpleProcessorExporterNames) {
LogRecordExporter exporter = exportersByNameCopy.remove(simpleProcessorExporterName);
if (exporter != null) {
LogRecordProcessor logRecordProcessor = SimpleLogRecordProcessor.create(exporter);
closeables.add(logRecordProcessor);
logRecordProcessors.add(logRecordProcessor);
}
}

if (!exportersByNameCopy.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ final class MetricExporterConfiguration {

static {
EXPORTER_ARTIFACT_ID_BY_NAME = new HashMap<>();
EXPORTER_ARTIFACT_ID_BY_NAME.put("console", "opentelemetry-exporter-logging");
EXPORTER_ARTIFACT_ID_BY_NAME.put("logging", "opentelemetry-exporter-logging");
EXPORTER_ARTIFACT_ID_BY_NAME.put("logging-otlp", "opentelemetry-exporter-logging-otlp");
EXPORTER_ARTIFACT_ID_BY_NAME.put("otlp", "opentelemetry-exporter-otlp");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ final class SpanExporterConfiguration {

static {
EXPORTER_ARTIFACT_ID_BY_NAME = new HashMap<>();
EXPORTER_ARTIFACT_ID_BY_NAME.put("console", "opentelemetry-exporter-logging");
EXPORTER_ARTIFACT_ID_BY_NAME.put("jaeger", "opentelemetry-exporter-jaeger");
EXPORTER_ARTIFACT_ID_BY_NAME.put("logging", "opentelemetry-exporter-logging");
EXPORTER_ARTIFACT_ID_BY_NAME.put("logging-otlp", "opentelemetry-exporter-logging-otlp");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.io.Closeable;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -32,6 +33,8 @@ final class TracerProviderConfiguration {

private static final double DEFAULT_TRACEIDRATIO_SAMPLE_RATIO = 1.0d;
private static final String PARENTBASED_ALWAYS_ON = "parentbased_always_on";
private static final List<String> simpleProcessorExporterNames =
Arrays.asList("console", "logging");

static void configureTracerProvider(
SdkTracerProviderBuilder tracerProviderBuilder,
Expand Down Expand Up @@ -65,11 +68,13 @@ static List<SpanProcessor> configureSpanProcessors(
Map<String, SpanExporter> exportersByNameCopy = new HashMap<>(exportersByName);
List<SpanProcessor> spanProcessors = new ArrayList<>();

SpanExporter exporter = exportersByNameCopy.remove("logging");
if (exporter != null) {
SpanProcessor spanProcessor = SimpleSpanProcessor.create(exporter);
closeables.add(spanProcessor);
spanProcessors.add(spanProcessor);
for (String simpleProcessorExporterNames : simpleProcessorExporterNames) {
SpanExporter exporter = exportersByNameCopy.remove(simpleProcessorExporterNames);
if (exporter != null) {
SpanProcessor spanProcessor = SimpleSpanProcessor.create(exporter);
closeables.add(spanProcessor);
spanProcessors.add(spanProcessor);
}
}

if (!exportersByNameCopy.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.net.URLClassLoader;
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import org.assertj.core.api.InstanceOfAssertFactories;
Expand Down Expand Up @@ -137,20 +138,25 @@ void configureExporter_NotFound() {

@Test
void configureSpanProcessors_simpleSpanProcessor() {
String exporterName = "logging";
List<Closeable> closeables = new ArrayList<>();

Map<String, SpanExporter> exportersByName = new LinkedHashMap<>();
exportersByName.put("console", LoggingSpanExporter.create());
exportersByName.put("logging", LoggingSpanExporter.create());

List<SpanProcessor> spanProcessors =
TracerProviderConfiguration.configureSpanProcessors(
DefaultConfigProperties.createFromMap(
Collections.singletonMap("otel.traces.exporter", exporterName)),
ImmutableMap.of(exporterName, LoggingSpanExporter.create()),
Collections.singletonMap("otel.traces.exporter", "console,logging")),
exportersByName,
MeterProvider.noop(),
closeables);
cleanup.addCloseables(closeables);

assertThat(spanProcessors).hasExactlyElementsOfTypes(SimpleSpanProcessor.class);
assertThat(closeables).hasExactlyElementsOfTypes(SimpleSpanProcessor.class);
assertThat(spanProcessors)
.hasExactlyElementsOfTypes(SimpleSpanProcessor.class, SimpleSpanProcessor.class);
assertThat(closeables)
.hasExactlyElementsOfTypes(SimpleSpanProcessor.class, SimpleSpanProcessor.class);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ void configureExporter_KnownSpiExportersOnClasspath() {
LogRecordExporterConfiguration.logRecordExporterSpiManager(
DefaultConfigProperties.createFromMap(Collections.emptyMap()), spiHelper);

assertThat(LogRecordExporterConfiguration.configureExporter("console", spiExportersManager))
.isInstanceOf(SystemOutLogRecordExporter.class);
assertThat(LogRecordExporterConfiguration.configureExporter("logging", spiExportersManager))
.isInstanceOf(SystemOutLogRecordExporter.class);
assertThat(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import static org.assertj.core.api.Assertions.assertThat;

import com.google.common.collect.ImmutableMap;
import io.opentelemetry.api.metrics.MeterProvider;
import io.opentelemetry.exporter.logging.SystemOutLogRecordExporter;
import io.opentelemetry.exporter.otlp.logs.OtlpGrpcLogRecordExporter;
Expand All @@ -18,12 +17,14 @@
import io.opentelemetry.sdk.logs.SdkLoggerProvider;
import io.opentelemetry.sdk.logs.SdkLoggerProviderBuilder;
import io.opentelemetry.sdk.logs.export.BatchLogRecordProcessor;
import io.opentelemetry.sdk.logs.export.LogRecordExporter;
import io.opentelemetry.sdk.logs.export.SimpleLogRecordProcessor;
import io.opentelemetry.sdk.trace.internal.JcTools;
import java.io.Closeable;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Queue;
Expand Down Expand Up @@ -84,26 +85,31 @@ void configureLoggerProvider() {
void configureLogRecordProcessors_multipleExportersWithLogging() {
List<Closeable> closeables = new ArrayList<>();

Map<String, LogRecordExporter> exportersByName = new LinkedHashMap<>();
exportersByName.put("console", SystemOutLogRecordExporter.create());
exportersByName.put("logging", SystemOutLogRecordExporter.create());
exportersByName.put("otlp", OtlpGrpcLogRecordExporter.builder().build());

List<LogRecordProcessor> logRecordProcessors =
LoggerProviderConfiguration.configureLogRecordProcessors(
DefaultConfigProperties.createFromMap(Collections.emptyMap()),
ImmutableMap.of(
"logging",
SystemOutLogRecordExporter.create(),
"otlp",
OtlpGrpcLogRecordExporter.builder().build()),
exportersByName,
MeterProvider.noop(),
closeables);
cleanup.addCloseables(closeables);

assertThat(logRecordProcessors)
.hasSize(2)
.hasAtLeastOneElementOfType(SimpleLogRecordProcessor.class)
.hasAtLeastOneElementOfType(BatchLogRecordProcessor.class);
.hasSize(3)
.hasExactlyElementsOfTypes(
SimpleLogRecordProcessor.class,
SimpleLogRecordProcessor.class,
BatchLogRecordProcessor.class);
assertThat(closeables)
.hasSize(2)
.hasAtLeastOneElementOfType(SimpleLogRecordProcessor.class)
.hasAtLeastOneElementOfType(BatchLogRecordProcessor.class);
.hasSize(3)
.hasExactlyElementsOfTypes(
SimpleLogRecordProcessor.class,
SimpleLogRecordProcessor.class,
BatchLogRecordProcessor.class);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ void configureExporter_KnownSpiExportersOnClasspath() {

assertThat(SpanExporterConfiguration.configureExporter("jaeger", spiExportersManager))
.isInstanceOf(io.opentelemetry.exporter.jaeger.JaegerGrpcSpanExporter.class);
assertThat(SpanExporterConfiguration.configureExporter("console", spiExportersManager))
.isInstanceOf(LoggingSpanExporter.class);
assertThat(SpanExporterConfiguration.configureExporter("logging", spiExportersManager))
.isInstanceOf(LoggingSpanExporter.class);
assertThat(SpanExporterConfiguration.configureExporter("logging-otlp", spiExportersManager))
Expand Down