Skip to content

[File based config] Support for default profiler's log ingest URL in declarative config #2518

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.

Sign up for GitHub

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

Merged
robsunday merged 13 commits into from
Nov 6, 2025
Merged

[File based config] Support for default profiler's log ingest URL in declarative config #2518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
87f1339
Initial configuration refactoring
robsunday Oct 29, 2025
480405a
Additional cleanup of Configuration class.
robsunday Oct 29, 2025
41cc08e
Additional cleanup of Configuration class.
robsunday Oct 29, 2025
c56343d
Revert "Additional cleanup of Configuration class."
robsunday Oct 29, 2025
3695261
Spotless
robsunday Oct 29, 2025
43bd074
Tests fixed
robsunday Oct 30, 2025
bc840e9
Update profiler/src/main/java/com/splunk/opentelemetry/profiler/Confi…
robsunday Nov 3, 2025
c98a601
Code review followup
robsunday Nov 3, 2025
8dd45fe
Fixed issue with getConfigUrl() method
robsunday Nov 3, 2025
d1ec23b
Reverting an unnecessary change
robsunday Nov 3, 2025
838c47a
Merge branch 'main' into fbc-profiler
robsunday Nov 3, 2025
a6d691d
Merge branch 'main' into fbc-profiler
robsunday Nov 4, 2025
8196c59
Refactoring to let Configuration.getIngestUrl(config) return not null…
robsunday Nov 5, 2025
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
43 changes: 28 additions & 15 deletions profiler/src/main/java/com/splunk/opentelemetry/profiler/Configuration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public static void log(ConfigProperties config) {
log(CONFIG_KEY_RECORDING_DURATION, getRecordingDuration(config).toMillis() + "ms");
log(CONFIG_KEY_KEEP_FILES, getKeepFiles(config));
log(CONFIG_KEY_PROFILER_OTLP_PROTOCOL, getOtlpProtocol(config));
log(CONFIG_KEY_INGEST_URL, getConfigUrl(config));
log(CONFIG_KEY_INGEST_URL, getIngestUrl(config));
log(CONFIG_KEY_OTEL_OTLP_URL, config.getString(CONFIG_KEY_OTEL_OTLP_URL));
log(CONFIG_KEY_MEMORY_ENABLED, getMemoryEnabled(config));
if (getMemoryEventRateLimitEnabled(config)) {
Expand All @@ -85,27 +85,40 @@ private static void log(String key, @Nullable Object value) {
logger.info(String.format("%39s : %s", key, value));
}

public static String getConfigUrl(ConfigProperties config) {
String ingestUrl = config.getString(CONFIG_KEY_OTEL_OTLP_URL);
if (ingestUrl != null) {
if (ingestUrl.startsWith("https://ingest.")
&& ingestUrl.endsWith(".signalfx.com")
&& config.getString(CONFIG_KEY_INGEST_URL) == null) {
public static String getIngestUrl(ConfigProperties config) {
String ingestUrl = config.getString(CONFIG_KEY_INGEST_URL);

if (ingestUrl == null) {
String defaultIngestUrl = getDefaultLogsEndpoint(config);
ingestUrl = config.getString(CONFIG_KEY_OTEL_OTLP_URL, defaultIngestUrl);

if (ingestUrl.startsWith("https://ingest.") && ingestUrl.endsWith(".signalfx.com")) {
logger.log(
WARNING,
"Profiling data can not be sent to {0}, using {1} instead. "
+ "You can override it by setting splunk.profiler.logs-endpoint",
new Object[] {ingestUrl, getDefaultLogsEndpoint(config)});
return null;
+ "You can override it by setting "
+ CONFIG_KEY_INGEST_URL,
new Object[] {ingestUrl, defaultIngestUrl});
return defaultIngestUrl;
Copy link
Copy Markdown
Contributor Author

@robsunday robsunday Nov 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

[For reviewer] It now returns default ingest URL instead of null, which is more consistent with the warning reported above.
Configuration.getIngestUrl() method is called only from LogExporterBuilder which then calls appropriate log exporter builder: OtlpGrpcLogRecordExporterBuilder or OtlpHttpLogRecordExporterBuilder. These use their own default URLs in case endpoint is undefined, but they are basically identical to these returned from Configuration.getIngestUrl() now.

}

if ("http/protobuf".equals(getOtlpProtocol(config))) {
if (!ingestUrl.endsWith("/")) {
ingestUrl += "/";
}
ingestUrl += "v1/logs";
ingestUrl = maybeAppendHttpPath(ingestUrl);
}
}
return config.getString(CONFIG_KEY_INGEST_URL, ingestUrl);

return ingestUrl;
}

private static String maybeAppendHttpPath(String ingestUrl) {
if (!ingestUrl.endsWith("v1/logs")) {
if (!ingestUrl.endsWith("/")) {
ingestUrl += "/";
}
ingestUrl += "v1/logs";
}

return ingestUrl;
}

private static String getDefaultLogsEndpoint(ConfigProperties config) {
Expand Down
14 changes: 6 additions & 8 deletions profiler/src/main/java/com/splunk/opentelemetry/profiler/LogExporterBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,17 @@ static LogRecordExporter fromConfig(ConfigProperties config) {
static LogRecordExporter buildGrpcExporter(
ConfigProperties config, Supplier<OtlpGrpcLogRecordExporterBuilder> makeBuilder) {
OtlpGrpcLogRecordExporterBuilder builder = makeBuilder.get();
String ingestUrl = Configuration.getConfigUrl(config);
if (ingestUrl != null) {
Copy link
Copy Markdown
Contributor Author

@robsunday robsunday Nov 5, 2025

Choose a reason for hiding this comment

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

[for reviewer] No need for null check now

builder.setEndpoint(ingestUrl);
}
String ingestUrl = Configuration.getIngestUrl(config);
Copy link
Copy Markdown
Collaborator

@laurit laurit Nov 5, 2025

Choose a reason for hiding this comment

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

Perhaps it would be better to pass the ingest url to this method instead of the config?

Copy link
Copy Markdown
Contributor Author

@robsunday robsunday Nov 6, 2025

Choose a reason for hiding this comment

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

Your suggestion looks good, however now we have consistent method signatures for buildHttpExporter and buildGrpcExporter methods which improves readability. I'll merge this PR and then probably make a followup.

builder.setEndpoint(ingestUrl);

return builder.addHeader(EXTRA_CONTENT_TYPE, STACKTRACES_HEADER_VALUE).build();
}

@VisibleForTesting
static LogRecordExporter buildHttpExporter(
ConfigProperties config, Supplier<OtlpHttpLogRecordExporterBuilder> makeBuilder) {
OtlpHttpLogRecordExporterBuilder builder = makeBuilder.get();
String ingestUrl = Configuration.getConfigUrl(config);
String ingestUrl = Configuration.getIngestUrl(config);

OtlpConfigUtil.configureOtlpExporterBuilder(
DATA_TYPE_LOGS,
Expand All @@ -73,9 +72,8 @@ static LogRecordExporter buildHttpExporter(
builder::setRetryPolicy,
builder::setMemoryMode);

if (ingestUrl != null) {
Copy link
Copy Markdown
Contributor Author

@robsunday robsunday Nov 5, 2025

Choose a reason for hiding this comment

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

[for reviewer] No need for null check now

builder.setEndpoint(ingestUrl);
}
builder.setEndpoint(ingestUrl);

return builder.addHeader(EXTRA_CONTENT_TYPE, STACKTRACES_HEADER_VALUE).build();
}
}
106 changes: 80 additions & 26 deletions profiler/src/test/java/com/splunk/opentelemetry/profiler/ConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@

package com.splunk.opentelemetry.profiler;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand All @@ -35,52 +37,98 @@ class ConfigurationTest {

String logsEndpoint = "http://logs.example.com";
String otelEndpoint = "http://otel.example.com";
String defaultLogsEndpoint = "http://localhost:4318/v1/logs";

@Test
void getConfigUrl_endpointDefined() {
void getIngestUrl_endpointDefined() {
// given
ConfigProperties config = mock(ConfigProperties.class);
when(config.getString(Configuration.CONFIG_KEY_OTEL_OTLP_URL)).thenReturn(otelEndpoint);
when(config.getString(Configuration.CONFIG_KEY_INGEST_URL, otelEndpoint))
.thenReturn(logsEndpoint);
String result = Configuration.getConfigUrl(config);
assertEquals(logsEndpoint, result);
when(config.getString(Configuration.CONFIG_KEY_INGEST_URL)).thenReturn(logsEndpoint);

// when
String result = Configuration.getIngestUrl(config);

// then
assertThat(result).isEqualTo(logsEndpoint);
}

@Test
void getIngestUrl_endpointNotDefined_usedOtelGrpc() {
// given
ConfigProperties config = mock(ConfigProperties.class);
when(config.getString(Configuration.CONFIG_KEY_INGEST_URL)).thenReturn(null);
when(config.getString(eq(Configuration.CONFIG_KEY_OTEL_OTLP_URL), anyString()))
.thenReturn(otelEndpoint);
when(config.getString(eq(Configuration.CONFIG_KEY_PROFILER_OTLP_PROTOCOL), any()))
.thenReturn("grpc");

// when
String result = Configuration.getIngestUrl(config);

// then
assertThat(result).isEqualTo(otelEndpoint);
}

@Test
void getConfigUrl_endpointNotDefined() {
void getIngestUrl_endpointNotDefined_usedOtelHttpProtobuf() {
// given
ConfigProperties config = mock(ConfigProperties.class);
when(config.getString(Configuration.CONFIG_KEY_OTEL_OTLP_URL)).thenReturn(otelEndpoint);
when(config.getString(Configuration.CONFIG_KEY_INGEST_URL, otelEndpoint))
when(config.getString(Configuration.CONFIG_KEY_INGEST_URL)).thenReturn(null);
when(config.getString(eq(Configuration.CONFIG_KEY_OTEL_OTLP_URL), anyString()))
.thenReturn(otelEndpoint);
String result = Configuration.getConfigUrl(config);
assertEquals(otelEndpoint, result);
when(config.getString(eq(Configuration.CONFIG_KEY_PROFILER_OTLP_PROTOCOL), any()))
.thenReturn("http/protobuf");

// when
String result = Configuration.getIngestUrl(config);

// then
assertThat(result).isEqualTo(otelEndpoint + "/v1/logs");
}

@Test
void getConfigUrlNull() {
void getIngestUrl_endpointNotDefined_usedOtelHttpProtobufWithPath() {
// given
String endpoint = otelEndpoint + "/v1/logs";

ConfigProperties config = mock(ConfigProperties.class);
when(config.getString(Configuration.CONFIG_KEY_OTEL_OTLP_URL, null)).thenReturn(null);
when(config.getString(Configuration.CONFIG_KEY_INGEST_URL, null)).thenReturn(null);
String result = Configuration.getConfigUrl(config);
assertNull(result);
when(config.getString(Configuration.CONFIG_KEY_INGEST_URL)).thenReturn(null);
when(config.getString(eq(Configuration.CONFIG_KEY_OTEL_OTLP_URL), anyString()))
.thenReturn(endpoint);
when(config.getString(eq(Configuration.CONFIG_KEY_PROFILER_OTLP_PROTOCOL), any()))
.thenReturn("http/protobuf");

// when
String result = Configuration.getIngestUrl(config);

// then
assertThat(result).isEqualTo(endpoint);
}

@Test
void getConfigUrlSplunkRealm() {
void getIngestUrlSplunkRealm() {
// given
ConfigProperties config = mock(ConfigProperties.class);
when(config.getString(Configuration.CONFIG_KEY_OTEL_OTLP_URL))
when(config.getString(Configuration.CONFIG_KEY_INGEST_URL)).thenReturn(null);
when(config.getString(eq(Configuration.CONFIG_KEY_OTEL_OTLP_URL), anyString()))
.thenReturn("https://ingest.us0.signalfx.com");
when(config.getString(Configuration.CONFIG_KEY_INGEST_URL, null)).thenReturn(null);
String result = Configuration.getConfigUrl(config);
assertNull(result);
when(config.getString(eq(Configuration.CONFIG_KEY_PROFILER_OTLP_PROTOCOL), any()))
.thenReturn("http/protobuf");

// when
String result = Configuration.getIngestUrl(config);

// then
assertThat(result).isEqualTo(defaultLogsEndpoint);
}

@Test
void getOtlpProtocolDefault() {
String result =
Configuration.getOtlpProtocol(
DefaultConfigProperties.create(Collections.emptyMap(), COMPONENT_LOADER));
assertEquals("http/protobuf", result);

assertThat(result).isEqualTo("http/protobuf");
}

@Test
Expand All @@ -89,16 +137,22 @@ void getOtlpProtocolOtelPropertySet() {
Configuration.getOtlpProtocol(
DefaultConfigProperties.create(
Collections.singletonMap("otel.exporter.otlp.protocol", "test"), COMPONENT_LOADER));
assertEquals("test", result);

assertThat(result).isEqualTo("test");
}

@Test
void getOtlpProtocol() {
// given
Map<String, String> map = new HashMap<>();
map.put("otel.exporter.otlp.protocol", "test1");
map.put("splunk.profiler.otlp.protocol", "test2");

// when
String result =
Configuration.getOtlpProtocol(DefaultConfigProperties.create(map, COMPONENT_LOADER));
assertEquals("test2", result);

// then
assertThat(result).isEqualTo("test2");
}
}
Loading