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
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,22 @@ public static List<String> httpServerResponseCapturedHeaders(ConfigProvider conf
"server");
}

/**
* Return {@code .instrumentation.general.sanitization.url.sensitive_query_parameters}, or null if
* none is configured.
*
* @throws DeclarativeConfigException if an unexpected type is encountered accessing the property
*/
@Nullable
public static List<String> sensitiveQueryParameters(ConfigProvider configProvider) {
Copy link
Member

Choose a reason for hiding this comment

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

Do you like these helper functions?

Copy link
Member Author

Choose a reason for hiding this comment

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

good question, I guess not really 😄

Copy link
Member

Choose a reason for hiding this comment

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

My initial thought is that they would be helpful for instrumentation conform to these general config options. All the instrumentation in opentelemetry-java-instrumentation can leverage shared utils in that repo to do this type of thing, so its really only native instrumentation that would stand to benefit.

Copy link
Member Author

Choose a reason for hiding this comment

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

I kind of like the explicitness of seeing the yaml path in the code.

But can see them being useful.

What's your thought about these methods handling default values (and never returning null)?

return getOrNull(
configProvider,
config -> config.getScalarList("sensitive_query_parameters", String.class),
"general",
"sanitization",
"url");
}

/**
* Return {@code .instrumentation.java.<instrumentationName>}, or null if none is configured.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ class InstrumentationConfigUtilTest {
+ " response_captured_headers:\n"
+ " - server-response-header1\n"
+ " - server-response-header2\n"
+ " sanitization:\n"
+ " url:\n"
+ " sensitive_query_parameters:\n"
+ " - AWSAccessKeyId\n"
+ " - Signature\n"
+ " - sig\n"
+ " - X-Goog-Signature\n"
+ " java:\n"
+ " example:\n"
+ " property: \"value\"";
Expand All @@ -58,6 +65,8 @@ class InstrumentationConfigUtilTest {
toConfigProvider("instrumentation/development:\n general:\n");
private static final ConfigProvider emptyHttpConfigProvider =
toConfigProvider("instrumentation/development:\n general:\n http:\n");
private static final ConfigProvider emptySanitizationConfigProvider =
toConfigProvider("instrumentation/development:\n general:\n sanitization:\n");

private static ConfigProvider toConfigProvider(String configYaml) {
return SdkConfigProvider.create(
Expand Down Expand Up @@ -139,6 +148,19 @@ void httpServerResponseCapturedHeaders() {
.isNull();
}

@Test
void sensitiveQueryParameters() {
assertThat(InstrumentationConfigUtil.sensitiveQueryParameters(kitchenSinkConfigProvider))
.isEqualTo(Arrays.asList("AWSAccessKeyId", "Signature", "sig", "X-Goog-Signature"));
assertThat(
InstrumentationConfigUtil.sensitiveQueryParameters(emptyInstrumentationConfigProvider))
.isNull();
assertThat(InstrumentationConfigUtil.sensitiveQueryParameters(emptyGeneralConfigProvider))
.isNull();
assertThat(InstrumentationConfigUtil.sensitiveQueryParameters(emptySanitizationConfigProvider))
.isNull();
}

@Test
void javaInstrumentationConfig() {
assertThat(
Expand Down
Loading