Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions opamp/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ dependencies {
compileOnly("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi")
compileOnly("io.opentelemetry.javaagent:opentelemetry-javaagent-extension-api")
compileOnly("org.snakeyaml:snakeyaml-engine")
compileOnly("com.squareup.okhttp3:okhttp")

annotationProcessor("com.google.auto.service:auto-service")
compileOnly("com.google.auto.service:auto-service")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,12 @@
import java.time.Duration;
import java.time.Instant;
import java.util.logging.Logger;
import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import opamp.proto.ComponentHealth;
import opamp.proto.ServerErrorResponse;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

@AutoService(AgentListener.class)
Expand Down Expand Up @@ -125,10 +129,10 @@ static OpampClient startOpampClient(
if (endpoint != null) {
PeriodicDelay pollingDelay =
PeriodicDelay.ofFixedDuration(Duration.ofMillis(pollingDurationMillis));
OkHttpSender okhttp = OkHttpSender.create(endpoint);
HttpRequestService httpSender =
OkHttpSender okhttp = buildOkHttpSender(opampClientConfiguration);
HttpRequestService httpRequestService =
HttpRequestService.create(okhttp, pollingDelay, DEFAULT_DELAY_BETWEEN_RETRIES);
builder.setRequestService(httpSender);
builder.setRequestService(httpRequestService);
}

OpampAgentAttributes agentAttributes = new OpampAgentAttributes(resource);
Expand All @@ -141,6 +145,22 @@ static OpampClient startOpampClient(
return builder.build(callbacks);
}

@NotNull
private static OkHttpSender buildOkHttpSender(OpampClientConfiguration config) {
String token = config.getAccessToken();
if (token == null) {
return OkHttpSender.create(config.getEndpoint());
}
Interceptor tokenFlinger =
chain -> {
Request.Builder requestBuilder = chain.request().newBuilder();
requestBuilder.addHeader("X-SF-TOKEN", token);
return chain.proceed(requestBuilder.build());
};
OkHttpClient okhttpClient = new OkHttpClient.Builder().addInterceptor(tokenFlinger).build();
return OkHttpSender.create(config.getEndpoint(), okhttpClient);
}

private static ComponentHealth createInitialHealthReport() {
Instant now = Instant.now();
long nowNanos = now.getEpochSecond() * 1_000_000_000L + now.getNano();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
public class OpampClientConfiguration {
private boolean enabled;
private String endpoint;
private String accessToken;
private long pollingInterval;

private OpampClientConfiguration() {}
Expand All @@ -39,6 +40,10 @@ public long getPollingInterval() {
return pollingInterval;
}

public String getAccessToken() {
return accessToken;
}

@Override
public String toString() {
return "OpampClientConfiguration{"
Expand All @@ -48,6 +53,9 @@ public String toString() {
+ ", endpoint='"
+ endpoint
+ '\''
+ ", accessToken='"
+ (accessToken == null ? "<null>" : "***redacted***")
+ '\''
+ ", pollingInterval="
+ pollingInterval
+ '}';
Expand All @@ -73,6 +81,11 @@ public Builder withPollingInterval(long pollingInterval) {
return this;
}

public Builder withAccessToken(String accessToken) {
configuredInstance.accessToken = accessToken;
return this;
}

public OpampClientConfiguration build() {
return configuredInstance;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.splunk.opentelemetry.opamp;

import static com.splunk.opentelemetry.SplunkConfigurationCustomizer.SPLUNK_ACCESS_TOKEN;
import static io.opentelemetry.opamp.client.internal.request.service.HttpRequestService.DEFAULT_DELAY_BETWEEN_REQUESTS;

import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
Expand Down Expand Up @@ -47,6 +48,7 @@ private static OpampClientConfiguration createConfigurationFromDeclarativeConfig
builder.withEnabled(true);
builder
.withEndpoint(opampProperties.getString("endpoint"))
.withAccessToken(opampProperties.getString("access_token"))
.withPollingInterval(
opampProperties.getLong(
"polling_interval", DEFAULT_DELAY_BETWEEN_REQUESTS.getNextDelay().toMillis()));
Expand All @@ -59,6 +61,7 @@ private static OpampClientConfiguration createConfiguration(ConfigProperties con
return OpampClientConfiguration.builder()
.withEnabled(config.getBoolean("splunk.opamp.enabled", false))
.withEndpoint(config.getString("splunk.opamp.endpoint"))
.withAccessToken(config.getString(SPLUNK_ACCESS_TOKEN))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should either use constants in all places here, or just a plain strings. This constant is not used anywhere else, so to me it is not worth making this class bigger by adding a constant.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That constant is used in a few other places. I prefer using existing constants over duplicating string literals, which is prone to creating inconsistencies. I didn't create constants for the OTHER string key literals because they are only used here.

.withPollingInterval(
config.getLong(
"splunk.opamp.polling.interval",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ void testOpamp() throws Exception {
OpampClientConfiguration.builder()
.withEnabled(true)
.withEndpoint(server.httpUri().toString())
.withAccessToken("test-access-token")
.withPollingInterval(500)
.build();
OpampClient client =
Expand Down Expand Up @@ -177,6 +178,8 @@ public void onMessage(OpampClient opampClient, MessageData messageData) {
assertThat(remoteConfig.config.config_map.get("test-key").body.utf8()).isEqualTo("test-value");

RecordedRequest recordedRequest = server.takeRequest();
assertThat(recordedRequest.request().headers().get("X-SF-TOKEN"))
.isEqualTo("test-access-token");
byte[] body = recordedRequest.request().content().array();
AgentToServer agentToServer = AgentToServer.ADAPTER.decode(body);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ void shouldCreateConfigurationFromEnvVars() {
Map.of(
"splunk.opamp.enabled", "true",
"splunk.opamp.endpoint", "https://opamp.example.com",
"splunk.access.token", "env-token",
"splunk.opamp.polling.interval", "3210"));

// when
Expand All @@ -55,6 +56,7 @@ void shouldCreateConfigurationFromEnvVars() {
// then
assertThat(configuration.isEnabled()).isTrue();
assertThat(configuration.getEndpoint()).isEqualTo("https://opamp.example.com");
assertThat(configuration.getAccessToken()).isEqualTo("env-token");
assertThat(configuration.getPollingInterval()).isEqualTo(3210);
}

Expand All @@ -68,6 +70,7 @@ void shouldCreateConfigurationFromDeclarativeConfig(@TempDir Path tempDir) throw
splunk:
opamp/development:
endpoint: http://some.opamp-host.com:3420/v1/opamp
access_token: declarative-token
polling_interval: 4567
""";
AutoConfiguredOpenTelemetrySdk sdk =
Expand All @@ -80,6 +83,7 @@ void shouldCreateConfigurationFromDeclarativeConfig(@TempDir Path tempDir) throw
// then
assertThat(configuration.isEnabled()).isTrue();
assertThat(configuration.getEndpoint()).isEqualTo("http://some.opamp-host.com:3420/v1/opamp");
assertThat(configuration.getAccessToken()).isEqualTo("declarative-token");
assertThat(configuration.getPollingInterval()).isEqualTo(4567);
}

Expand All @@ -95,6 +99,7 @@ void shouldUseDefaults() {
// then
assertThat(configuration.isEnabled()).isFalse();
assertThat(configuration.getEndpoint()).isNull();
assertThat(configuration.getAccessToken()).isNull();
assertThat(configuration.getPollingInterval())
.isEqualTo(DEFAULT_DELAY_BETWEEN_REQUESTS.getNextDelay().toMillis());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright Splunk Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.splunk.opentelemetry.opamp;

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

import org.junit.jupiter.api.Test;

class OpampClientConfigurationTest {

@Test
void shouldConfigureAccessToken() {
OpampClientConfiguration configuration =
OpampClientConfiguration.builder().withAccessToken("secret-token").build();

assertThat(configuration.getAccessToken()).isEqualTo("secret-token");
assertThat(configuration.toString())
.contains("accessToken='***redacted***'")
.doesNotContain("secret-token");
}

@Test
void shouldDefaultAccessTokenToNull() {
OpampClientConfiguration configuration = OpampClientConfiguration.builder().build();

assertThat(configuration.getAccessToken()).isNull();
assertThat(configuration.toString()).contains("accessToken='<null>'");
}
}
Loading