Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@lmolkova
lmolkova committed Jan 22, 2025
1 parent a84da73 commit 623de4c
Show file tree
Hide file tree
Showing 9 changed files with 160 additions and 97 deletions.
158 changes: 100 additions & 58 deletions ...entcore/core/src/main/java/io/clientcore/core/http/models/HttpInstrumentationOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
* <li>Error details if the request fails</li>
* <li>Time it takes to receive response</li>
* <li>Correlation identifiers</li>
* <li>When content logging is enabled via {@link #setContentLoggingEnabled(boolean)}: request and response body, and time-to-last-byte</li>
* <li>When content logging is enabled via {@link HttpLogDetailLevel#BODY_AND_HEADERS}: request and response body, and time-to-last-byte</li>
* </ul>
*
* Client libraries auto-discover global OpenTelemetry SDK instance configured by the java agent or
Expand Down Expand Up @@ -80,8 +80,7 @@
* <!-- end io.clientcore.core.telemetry.useexplicitopentelemetry -->
*/
public final class HttpInstrumentationOptions extends InstrumentationOptions {
private boolean isHttpLoggingEnabled;
private boolean isContentLoggingEnabled;
private HttpLogDetailLevel logDetailLevel;
private boolean isRedactedHeaderNamesLoggingEnabled;
private Set<HttpHeaderName> allowedHeaderNames;
private Set<String> allowedQueryParamNames;
Expand All @@ -93,18 +92,10 @@ public final class HttpInstrumentationOptions extends InstrumentationOptions {
HttpHeaderName.PRAGMA, HttpHeaderName.RETRY_AFTER, HttpHeaderName.SERVER, HttpHeaderName.TRANSFER_ENCODING,
HttpHeaderName.USER_AGENT, HttpHeaderName.WWW_AUTHENTICATE);

static final HttpLogDetailLevel ENVIRONMENT_HTTP_LOG_DETAIL_LEVEL
= HttpLogDetailLevel.fromConfiguration(Configuration.getGlobalConfiguration());
private static final List<String> DEFAULT_QUERY_PARAMS_ALLOWLIST = Collections.singletonList("api-version");

private static final ConfigurationProperty<Boolean> HTTP_LOGGING_ENABLED
= ConfigurationPropertyBuilder.ofBoolean("http.logging.enabled")
.shared(true)
.environmentVariableName(Configuration.PROPERTY_HTTP_LOGGING_ENABLED)
.defaultValue(false)
.build();

private static final boolean DEFAULT_HTTP_LOGGING_ENABLED
= Configuration.getGlobalConfiguration().get(HTTP_LOGGING_ENABLED);

/**
* Creates a new instance using default options:
* <ul>
Expand All @@ -114,24 +105,22 @@ public final class HttpInstrumentationOptions extends InstrumentationOptions {
*/
public HttpInstrumentationOptions() {
super();
isHttpLoggingEnabled = DEFAULT_HTTP_LOGGING_ENABLED;
isContentLoggingEnabled = false;
logDetailLevel = ENVIRONMENT_HTTP_LOG_DETAIL_LEVEL;
isRedactedHeaderNamesLoggingEnabled = true;
allowedHeaderNames = new HashSet<>(DEFAULT_HEADERS_ALLOWLIST);
allowedQueryParamNames = new HashSet<>(DEFAULT_QUERY_PARAMS_ALLOWLIST);
}

/**
* Flag indicating whether detailed HTTP request and response logging is enabled.
* False by default.
* Gets the level of detail for HTTP request logs. Default is {@link HttpLogDetailLevel#NONE}.
* <p>
* When HTTP logging is disabled, basic information about the request and response is still recorded
* on distributed tracing spans.
*
* @return True if logging is enabled, false otherwise.
* @return The {@link HttpLogDetailLevel}.
*/
public boolean isHttpLoggingEnabled() {
return isHttpLoggingEnabled;
public HttpLogDetailLevel getHttpLogLevel() {
return logDetailLevel;
}

/**
Expand All @@ -158,46 +147,15 @@ public HttpInstrumentationOptions setRedactedHeaderNamesLoggingEnabled(boolean r
}

/**
* Flag indicating whether HTTP request and response body is logged.
* False by default.
* <p>
* Note: even when content logging is explicitly enabled, content is not logged
* for requests and responses where the content length is not known or greater than 16KB.
* Sets the level of detail for HTTP request logs.
* Default is {@link HttpLogDetailLevel#NONE}.
*
* @return True if content logging is enabled, false otherwise.
*/
public boolean isContentLoggingEnabled() {
return isContentLoggingEnabled;
}

/**
* Enables or disables logging of HTTP request and response.
* False by default.
* <p>
* When HTTP logging is disabled, basic information about the request and response is still recorded
* via distributed tracing.
*
* @param isHttpLoggingEnabled True to enable detailed HTTP logging, false otherwise.
* @return The updated {@link HttpInstrumentationOptions} object.
*/
public HttpInstrumentationOptions setHttpLoggingEnabled(boolean isHttpLoggingEnabled) {
this.isHttpLoggingEnabled = isHttpLoggingEnabled;
return this;
}

/**
* Enables or disables logging of HTTP request and response body. False by default.
* Enabling content logging also enables HTTP logging in general.
* <p>
* Note: even when content logging is explicitly enabled, content is not logged for requests and responses where the
* content length is not known or greater than 16KB.
* @param logDetailLevel The {@link HttpLogDetailLevel}.
*
* @param isContentLoggingEnabled True to enable content logging, false otherwise.
* @return The updated {@link HttpInstrumentationOptions} object.
*/
public HttpInstrumentationOptions setContentLoggingEnabled(boolean isContentLoggingEnabled) {
this.isHttpLoggingEnabled |= isContentLoggingEnabled;
this.isContentLoggingEnabled = isContentLoggingEnabled;
public HttpInstrumentationOptions setHttpLogLevel(HttpLogDetailLevel logDetailLevel) {
this.logDetailLevel = logDetailLevel;
return this;
}

Expand Down Expand Up @@ -226,7 +184,7 @@ public Set<HttpHeaderName> getAllowedHeaderNames() {
* @return The updated HttpLogOptions object.
*/
public HttpInstrumentationOptions setAllowedHeaderNames(final Set<HttpHeaderName> allowedHeaderNames) {
this.allowedHeaderNames = allowedHeaderNames == null ? new HashSet<>() : allowedHeaderNames;
this.allowedHeaderNames = allowedHeaderNames == null ? new HashSet<>() : new HashSet<>(allowedHeaderNames);

return this;
}
Expand Down Expand Up @@ -266,7 +224,8 @@ public Set<String> getAllowedQueryParamNames() {
* @return The updated {@code allowedQueryParamName} object.
*/
public HttpInstrumentationOptions setAllowedQueryParamNames(final Set<String> allowedQueryParamNames) {
this.allowedQueryParamNames = allowedQueryParamNames == null ? new HashSet<>() : allowedQueryParamNames;
this.allowedQueryParamNames
= allowedQueryParamNames == null ? new HashSet<>() : new HashSet<>(allowedQueryParamNames);

return this;
}
Expand Down Expand Up @@ -296,4 +255,87 @@ public HttpInstrumentationOptions setTelemetryProvider(Object telemetryProvider)
super.setTelemetryProvider(telemetryProvider);
return this;
}

/**
* The level of detail for HTTP request logs.
*/
public enum HttpLogDetailLevel {
/**
* HTTP logging is turned off.
*/
NONE,

/**
* Enables logging the following information on detailed HTTP logs
* <ul>
* <li>Request method, URI, and body size. URI is sanitized based on allowed query parameters configurable with {@link #setAllowedQueryParamNames(Set)} and {@link #addAllowedQueryParamName(String)}</li>
* <li>Response status code and body size</li>
* <li>Request and response headers from allow-list configured via {@link #setAllowedHeaderNames(Set)} and {@link #addAllowedHeaderName(HttpHeaderName)}.</li>
* <li>Error details if the request fails</li>
* <li>Time it takes to receive response</li>
* <li>Correlation identifiers</li>
* </ul>
*/
HEADERS,

/**
* Enables logging the following information on detailed HTTP logs
* <ul>
* <li>Request method, URI, and body size. URI is sanitized based on allowed query parameters configurable with {@link #setAllowedQueryParamNames(Set)} and {@link #addAllowedQueryParamName(String)}</li>
* <li>Response status code and body size</li>
* <li>Error details if the request fails</li>
* <li>Time it takes to receive response</li>
* <li>Correlation identifiers</li>
* <li>Request and response bodies</li>
* <li>Time-to-last-byte</li>
* </ul>
*
* <p>
* The request and response body will be buffered into memory even if it is never consumed by an application, possibly impacting
* performance.
* <p>
* Body is not logged (and not buffered) for requests and responses where the content length is not known or greater than 16KB.
*/
BODY,

/**
* Enables logging everything in {@link #HEADERS} and {@link #BODY}.
*
* <p>
* The request and response body will be buffered into memory even if it is never consumed by an application, possibly impacting
* performance.
* <p>
* Body is not logged (and not buffered) for requests and responses where the content length is not known or greater than 16KB.
*/
BODY_AND_HEADERS;

private static final String HEADERS_VALUE = "headers";
private static final String BODY_VALUE = "body";
private static final String BODY_AND_HEADERS_VALUE = "body_and_headers";

private static final ConfigurationProperty<String> HTTP_LOG_DETAIL_LEVEL
= ConfigurationPropertyBuilder.ofString("http.log.detail.level")
.shared(true)
.environmentVariableName(Configuration.PROPERTY_HTTP_LOG_DETAIL_LEVEL)
.defaultValue("none")
.build();

static HttpLogDetailLevel fromConfiguration(Configuration configuration) {
String detailLevel = configuration.get(HTTP_LOG_DETAIL_LEVEL);

HttpLogDetailLevel logDetailLevel;

if (HEADERS_VALUE.equalsIgnoreCase(detailLevel)) {
logDetailLevel = HEADERS;
} else if (BODY_VALUE.equalsIgnoreCase(detailLevel)) {
logDetailLevel = BODY;
} else if (BODY_AND_HEADERS_VALUE.equalsIgnoreCase(detailLevel)) {
logDetailLevel = BODY_AND_HEADERS;
} else {
logDetailLevel = NONE;
}

return logDetailLevel;
}
}
}
6 changes: 4 additions & 2 deletions ...ntcore/core/src/main/java/io/clientcore/core/http/pipeline/HttpInstrumentationPolicy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,10 @@ public HttpInstrumentationPolicy(HttpInstrumentationOptions instrumentationOptio

HttpInstrumentationOptions optionsToUse
= instrumentationOptions == null ? DEFAULT_OPTIONS : instrumentationOptions;
this.isLoggingEnabled = optionsToUse.isHttpLoggingEnabled();
this.isContentLoggingEnabled = optionsToUse.isContentLoggingEnabled();
this.isLoggingEnabled = optionsToUse.getHttpLogLevel() != HttpInstrumentationOptions.HttpLogDetailLevel.NONE;
this.isContentLoggingEnabled
= optionsToUse.getHttpLogLevel() == HttpInstrumentationOptions.HttpLogDetailLevel.BODY
|| optionsToUse.getHttpLogLevel() == HttpInstrumentationOptions.HttpLogDetailLevel.BODY_AND_HEADERS;
this.isRedactedHeadersLoggingEnabled = optionsToUse.isRedactedHeaderNamesLoggingEnabled();
this.allowedHeaderNames = optionsToUse.getAllowedHeaderNames();
this.allowedQueryParameterNames = optionsToUse.getAllowedQueryParamNames()
Expand Down
2 changes: 1 addition & 1 deletion ...e/core/src/main/java/io/clientcore/core/implementation/util/EnvironmentConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class EnvironmentConfiguration {
*/
private static final Set<String> DEFAULT_CONFIGURATIONS
= new HashSet<>(Arrays.asList(Configuration.PROPERTY_HTTP_PROXY, Configuration.PROPERTY_HTTPS_PROXY,
Configuration.PROPERTY_LOG_LEVEL, Configuration.PROPERTY_HTTP_LOGGING_ENABLED,
Configuration.PROPERTY_LOG_LEVEL, Configuration.PROPERTY_HTTP_LOG_DETAIL_LEVEL,
Configuration.PROPERTY_REQUEST_RETRY_COUNT, Configuration.PROPERTY_REQUEST_CONNECT_TIMEOUT,
Configuration.PROPERTY_REQUEST_WRITE_TIMEOUT, Configuration.PROPERTY_REQUEST_RESPONSE_TIMEOUT,
Configuration.PROPERTY_REQUEST_READ_TIMEOUT));
Expand Down
2 changes: 1 addition & 1 deletion sdk/clientcore/core/src/main/java/io/clientcore/core/util/configuration/Configuration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class Configuration {
/**
* Enables HTTP request/response logging by setting an HTTP log detail level.
*/
public static final String PROPERTY_HTTP_LOGGING_ENABLED = "HTTP_LOGGING_ENABLED";
public static final String PROPERTY_HTTP_LOG_DETAIL_LEVEL = "HTTP_LOG_DETAIL_LEVEL";

/**
* Sets the default number of times a request will be retried, if it passes the conditions for retrying, before it
Expand Down
Loading

0 comments on commit 623de4c

Please sign in to comment.