-
Notifications
You must be signed in to change notification settings - Fork 1k
Support output_stream in declarative config of otlp_file/development #8676
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.exporter.logging.otlp.internal; | ||
|
|
||
| import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; | ||
| import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException; | ||
| import java.io.BufferedOutputStream; | ||
| import java.io.IOException; | ||
| import java.io.OutputStream; | ||
| import java.net.URI; | ||
| import java.net.URISyntaxException; | ||
| import java.nio.file.FileSystemNotFoundException; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.nio.file.Paths; | ||
| import java.nio.file.StandardOpenOption; | ||
| import java.util.function.Consumer; | ||
|
|
||
| /** | ||
| * Utilities for configuring the output stream of the OTLP file exporters. | ||
| * | ||
| * <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 OutputStreamConfigUtil { | ||
|
|
||
| private static final String STDOUT = "stdout"; | ||
| private static final String FILE_SCHEME = "file"; | ||
|
|
||
| /** | ||
| * Invoke the {@code outputStreamConsumer} with the configured output stream. | ||
| * | ||
| * <p>Recognized values are {@code stdout} and a file URI such as {@code | ||
| * file:///path/to/file.jsonl}. Missing parent directories of the file are created, and the file | ||
| * is appended to if it already exists. | ||
| */ | ||
| @SuppressWarnings("SystemOut") | ||
| public static void configureOutputStream( | ||
| DeclarativeConfigProperties config, Consumer<OutputStream> outputStreamConsumer) { | ||
| String outputStream = config.getString("output_stream"); | ||
| if (outputStream == null) { | ||
| return; | ||
| } | ||
| if (STDOUT.equalsIgnoreCase(outputStream)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for ignoring case. Better to be more explicit and case sensitive. Can always expand if needed. |
||
| outputStreamConsumer.accept(System.out); | ||
| return; | ||
| } | ||
| Path path = filePath(outputStream); | ||
| try { | ||
| Path parent = path.getParent(); | ||
| if (parent != null) { | ||
| Files.createDirectories(parent); | ||
| } | ||
| outputStreamConsumer.accept( | ||
| new BufferedOutputStream( | ||
| Files.newOutputStream(path, StandardOpenOption.CREATE, StandardOpenOption.APPEND))); | ||
| } catch (IOException e) { | ||
| throw new ConfigurationException("Unable to open output_stream: " + outputStream, e); | ||
| } | ||
| } | ||
|
|
||
| private static Path filePath(String outputStream) { | ||
| URI uri; | ||
| try { | ||
| uri = new URI(outputStream); | ||
| } catch (URISyntaxException e) { | ||
| throw new ConfigurationException("Unrecognized output_stream: " + outputStream, e); | ||
| } | ||
| if (!FILE_SCHEME.equalsIgnoreCase(uri.getScheme())) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here: match the scheme case-sensitively against |
||
| throw new ConfigurationException("Unrecognized output_stream: " + outputStream); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| } | ||
| try { | ||
| return Paths.get(uri); | ||
| } catch (IllegalArgumentException | FileSystemNotFoundException e) { | ||
| throw new ConfigurationException("Unrecognized output_stream: " + outputStream, e); | ||
| } | ||
| } | ||
|
|
||
| private OutputStreamConfigUtil() {} | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
| import io.github.netmikey.logunit.api.LogCapturer; | ||
| import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; | ||
| import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; | ||
| import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException; | ||
| import io.opentelemetry.sdk.autoconfigure.spi.internal.ComponentProvider; | ||
| import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; | ||
| import io.opentelemetry.sdk.common.export.MemoryMode; | ||
|
|
@@ -30,6 +31,7 @@ | |
| import java.nio.file.Path; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.function.Supplier; | ||
| import java.util.function.UnaryOperator; | ||
| import java.util.stream.Stream; | ||
| import java.util.stream.StreamSupport; | ||
| import javax.annotation.Nullable; | ||
|
|
@@ -302,6 +304,92 @@ void componentProviderConfig() { | |
| .isEqualTo(MemoryMode.REUSABLE_DATA); | ||
| } | ||
|
|
||
| static Stream<Arguments> outputStreamStdoutTestCases() { | ||
| return Stream.of( | ||
| Arguments.argumentSet("stdout", "stdout"), Arguments.argumentSet("upper case", "STDOUT")); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @MethodSource("outputStreamStdoutTestCases") | ||
| void componentProviderConfigOutputStreamStdout(String value) { | ||
| DeclarativeConfigProperties properties = spy(DeclarativeConfigProperties.empty()); | ||
| when(properties.getString("output_stream")).thenReturn(value); | ||
|
|
||
| assertThat(exporterFromComponentProvider(properties)) | ||
| .extracting("jsonWriter") | ||
| .extracting(Object::toString) | ||
| .isEqualTo("StreamJsonWriter{outputStream=stdout}"); | ||
| } | ||
|
|
||
| static Stream<Arguments> outputStreamFileTestCases() { | ||
| return Stream.of( | ||
| Arguments.argumentSet("file uri", "test.jsonl", (UnaryOperator<String>) uri -> uri), | ||
| Arguments.argumentSet( | ||
| "authority-less file uri with upper case scheme", | ||
| "test.jsonl", | ||
| (UnaryOperator<String>) uri -> uri.replaceFirst("^file://", "FILE:")), | ||
| Arguments.argumentSet( | ||
| "missing parent directory", "missing/test.jsonl", (UnaryOperator<String>) uri -> uri)); | ||
| } | ||
|
Comment on lines
+324
to
+333
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once matching is strict (see the |
||
|
|
||
| @ParameterizedTest | ||
| @MethodSource("outputStreamFileTestCases") | ||
| void componentProviderConfigOutputStreamFile(String relativePath, UnaryOperator<String> uriForm) | ||
| throws Exception { | ||
| Path file = tempDir.resolve(relativePath); | ||
| DeclarativeConfigProperties properties = spy(DeclarativeConfigProperties.empty()); | ||
| when(properties.getString("output_stream")).thenReturn(uriForm.apply(file.toUri().toString())); | ||
|
|
||
| T exporter = exporterFromComponentProvider(properties); | ||
| testDataExporter.export(exporter); | ||
|
|
||
| String output = new String(Files.readAllBytes(file), StandardCharsets.UTF_8).trim(); | ||
| JSONAssert.assertEquals( | ||
| "Got \n" + output, | ||
| testDataExporter.getExpectedJson(/* withWrapper= */ true), | ||
| output, | ||
| false); | ||
|
|
||
| // an exporter created later for the same path appends instead of truncating | ||
| testDataExporter.shutdown(exporter); | ||
| T secondExporter = exporterFromComponentProvider(properties); | ||
| testDataExporter.export(secondExporter); | ||
| testDataExporter.shutdown(secondExporter); | ||
| assertThat(new String(Files.readAllBytes(file), StandardCharsets.UTF_8).trim().split("\n")) | ||
| .hasSize(2); | ||
| } | ||
|
|
||
| static Stream<Arguments> outputStreamUnrecognizedTestCases() { | ||
| return Stream.of( | ||
| Arguments.argumentSet("no scheme", "not-a-stream"), | ||
| Arguments.argumentSet("unsupported scheme", "http://example.com/traces.jsonl"), | ||
| Arguments.argumentSet("relative file uri", "file://traces.jsonl"), | ||
| Arguments.argumentSet("malformed file uri", "file:///with space.jsonl")); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @MethodSource("outputStreamUnrecognizedTestCases") | ||
| void componentProviderConfigOutputStreamUnrecognized(String value) { | ||
| DeclarativeConfigProperties properties = spy(DeclarativeConfigProperties.empty()); | ||
| when(properties.getString("output_stream")).thenReturn(value); | ||
|
|
||
| assertThatExceptionOfType(ConfigurationException.class) | ||
| .isThrownBy(() -> exporterFromComponentProvider(properties)) | ||
| .withMessage("Unrecognized output_stream: " + value); | ||
| } | ||
|
|
||
| @Test | ||
| void componentProviderConfigOutputStreamNotOpenable() { | ||
| DeclarativeConfigProperties properties = spy(DeclarativeConfigProperties.empty()); | ||
| // the path exists but is a directory, so it cannot be opened for writing | ||
| when(properties.getString("output_stream")).thenReturn(tempDir.toUri().toString()); | ||
|
|
||
| assertThatExceptionOfType(ConfigurationException.class) | ||
| .isThrownBy(() -> exporterFromComponentProvider(properties)) | ||
| .withMessageStartingWith("Unable to open output_stream: ") | ||
| .withCauseInstanceOf(IOException.class); | ||
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| protected T exporterFromComponentProvider(DeclarativeConfigProperties properties) { | ||
| return (T) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't both adding a changelog entry. They're added as part of the release process. This is in the wrong section anyway.