Skip to content

Commit

Permalink
Further deprecate the handler API
Browse files Browse the repository at this point in the history
whenever the deprecated API is being used

- added logging of warnings
- added 5 second timeout

also removed any calls to the deprecated API from the existing integration tests. That's a breaking change in the way that we're not creating the default handlers in testing framework. I think that's fine cause we haven't setup any functionality for it neither have we documented any of its usage anywhere

fixes gh-371
  • Loading branch information
marcingrzejszczak committed Dec 8, 2023
1 parent a2c22c8 commit fce71d2
Show file tree
Hide file tree
Showing 20 changed files with 205 additions and 126 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,18 @@ public class BraveHttpClientHandler implements HttpClientHandler {
public BraveHttpClientHandler(
brave.http.HttpClientHandler<brave.http.HttpClientRequest, brave.http.HttpClientResponse> delegate) {
this.delegate = delegate;
DeprecatedClassLogger.logWarning(getClass());
}

@Override
public Span handleSend(HttpClientRequest request) {
DeprecatedClassLogger.logWarning(getClass());
return BraveSpan.fromBrave(this.delegate.handleSend(BraveHttpClientRequest.toBrave(request)));
}

@Override
public Span handleSend(HttpClientRequest request, TraceContext parent) {
DeprecatedClassLogger.logWarning(getClass());
brave.Span span = this.delegate.handleSendWithParent(BraveHttpClientRequest.toBrave(request),
BraveTraceContext.toBrave(parent));
if (!span.isNoop()) {
Expand All @@ -63,6 +66,7 @@ public Span handleSend(HttpClientRequest request, TraceContext parent) {

@Override
public void handleReceive(HttpClientResponse response, Span span) {
DeprecatedClassLogger.logWarning(getClass());
if (response == null) {
if (log.isDebugEnabled()) {
log.debug("Response is null, will not handle receiving of span [" + span + "]");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ public class BraveHttpRequestParser implements HttpRequestParser {
* Creates a new version of a {@link BraveHttpRequestParser}.
* @param delegate Brave version of {@link HttpRequestParser}
*/
@Deprecated
public BraveHttpRequestParser(brave.http.HttpRequestParser delegate) {
this.delegate = delegate;
DeprecatedClassLogger.logWarning(getClass());
}

/**
Expand All @@ -46,6 +48,7 @@ public BraveHttpRequestParser(brave.http.HttpRequestParser delegate) {
* @return Brave version of the parser
*/
public static brave.http.HttpRequestParser toBrave(HttpRequestParser parser) {
DeprecatedClassLogger.logWarning(BraveHttpRequestParser.class);
if (parser instanceof BraveHttpRequestParser) {
return ((BraveHttpRequestParser) parser).delegate;
}
Expand All @@ -55,6 +58,7 @@ public static brave.http.HttpRequestParser toBrave(HttpRequestParser parser) {

@Override
public void parse(HttpRequest request, TraceContext context, SpanCustomizer span) {
DeprecatedClassLogger.logWarning(getClass());
this.delegate.parse(BraveHttpRequest.toBrave(request), BraveTraceContext.toBrave(context),
BraveSpanCustomizer.toBrave(span));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public class BraveHttpResponseParser implements HttpResponseParser {
*/
public BraveHttpResponseParser(brave.http.HttpResponseParser delegate) {
this.delegate = delegate;
DeprecatedClassLogger.logWarning(getClass());
}

/**
Expand All @@ -46,6 +47,7 @@ public BraveHttpResponseParser(brave.http.HttpResponseParser delegate) {
* @return Brave parser
*/
public static brave.http.HttpResponseParser toBrave(HttpResponseParser parser) {
DeprecatedClassLogger.logWarning(BraveHttpRequestParser.class);
if (parser instanceof BraveHttpResponseParser) {
return ((BraveHttpResponseParser) parser).delegate;
}
Expand All @@ -55,6 +57,7 @@ public static brave.http.HttpResponseParser toBrave(HttpResponseParser parser) {

@Override
public void parse(HttpResponse response, TraceContext context, SpanCustomizer span) {
DeprecatedClassLogger.logWarning(getClass());
this.delegate.parse(BraveHttpResponse.toBrave(response), BraveTraceContext.toBrave(context),
BraveSpanCustomizer.toBrave(span));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,18 @@ public class BraveHttpServerHandler implements HttpServerHandler {
public BraveHttpServerHandler(
brave.http.HttpServerHandler<brave.http.HttpServerRequest, brave.http.HttpServerResponse> delegate) {
this.delegate = delegate;
DeprecatedClassLogger.logWarning(getClass());
}

@Override
public Span handleReceive(HttpServerRequest request) {
DeprecatedClassLogger.logWarning(getClass());
return BraveSpan.fromBrave(this.delegate.handleReceive(BraveHttpServerRequest.toBrave(request)));
}

@Override
public void handleSend(HttpServerResponse response, Span span) {
DeprecatedClassLogger.logWarning(getClass());
this.delegate.handleSend(BraveHttpServerResponse.toBrave(response), BraveSpan.toBrave(span));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
* @param <T> type of the input, for example a request or method
* @author Marcin Grzejszczak
* @since 1.0.0
* @deprecated scheduled for removal in 1.4.0
*/
@Deprecated
@SuppressWarnings("unchecked")
public final class BraveSamplerFunction<T> implements SamplerFunction<T> {

Expand All @@ -35,6 +37,7 @@ public final class BraveSamplerFunction<T> implements SamplerFunction<T> {
* @param samplerFunction Brave {@link SamplerFunction}
*/
public BraveSamplerFunction(brave.sampler.SamplerFunction<T> samplerFunction) {
DeprecatedClassLogger.logWarning(getClass());
this.samplerFunction = samplerFunction;
}

Expand All @@ -47,11 +50,13 @@ public BraveSamplerFunction(brave.sampler.SamplerFunction<T> samplerFunction) {
@Deprecated
public static brave.sampler.SamplerFunction<brave.http.HttpRequest> toHttpBrave(
SamplerFunction<HttpRequest> samplerFunction) {
DeprecatedClassLogger.logWarning(BraveSamplerFunction.class);
return arg -> samplerFunction.trySample(BraveHttpRequest.fromBrave(arg));
}

@Override
public Boolean trySample(T arg) {
DeprecatedClassLogger.logWarning(getClass());
return this.samplerFunction.trySample(arg);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* Copyright 2022 the original author or authors.
*
* 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
*
* https://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 io.micrometer.tracing.brave.bridge;

import io.micrometer.common.util.internal.logging.InternalLogger;
import io.micrometer.common.util.internal.logging.InternalLoggerFactory;

/**
* @deprecated scheduled for removal in 1.4.0
*/
@Deprecated
final class DeprecatedClassLogger {

private static final InternalLogger log = InternalLoggerFactory.getInstance(DeprecatedClassLogger.class);

static void logWarning(Class<?> clazz) {
log.warn("The class <{}> is scheduled for removal in the next minor. Please migrate away from using it.",
clazz);
try {
Thread.sleep(5_000);
}
catch (InterruptedException e) {
throw new RuntimeException(e);
}
}

}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,21 @@
public class DefaultHttpClientAttributesGetter
implements HttpClientAttributesGetter<HttpClientRequest, HttpClientResponse> {

public DefaultHttpClientAttributesGetter() {
DeprecatedClassLogger.logWarning(getClass());
}

@Nullable
@Override
public String getUrlFull(HttpClientRequest httpClientRequest) {
DeprecatedClassLogger.logWarning(getClass());
return httpClientRequest.url();
}

@Nullable
@Override
public String getServerAddress(HttpClientRequest httpClientRequest) {
DeprecatedClassLogger.logWarning(getClass());
try {
URI uri = URI.create(httpClientRequest.url());
return uri.getHost();
Expand All @@ -56,6 +62,7 @@ public String getServerAddress(HttpClientRequest httpClientRequest) {
@Nullable
@Override
public Integer getServerPort(HttpClientRequest httpClientRequest) {
DeprecatedClassLogger.logWarning(getClass());
try {
URI uri = URI.create(httpClientRequest.url());
return uri.getPort();
Expand All @@ -72,6 +79,7 @@ public Integer getServerPort(HttpClientRequest httpClientRequest) {
@Nullable
@Deprecated
public String getUrl(HttpClientRequest httpClientRequest) {
DeprecatedClassLogger.logWarning(getClass());
return this.getUrlFull(httpClientRequest);
}

Expand All @@ -82,12 +90,14 @@ public String getUrl(HttpClientRequest httpClientRequest) {
@Nullable
@Deprecated
public String getFlavor(HttpClientRequest httpClientRequest, @Nullable HttpClientResponse httpClientResponse) {
DeprecatedClassLogger.logWarning(getClass());
return null;
}

@Nullable
@Override
public String getHttpRequestMethod(HttpClientRequest httpClientRequest) {
DeprecatedClassLogger.logWarning(getClass());
return httpClientRequest.method();
}

Expand All @@ -98,11 +108,13 @@ public String getHttpRequestMethod(HttpClientRequest httpClientRequest) {
@Nullable
@Deprecated
public String getMethod(HttpClientRequest httpClientRequest) {
DeprecatedClassLogger.logWarning(getClass());
return this.getHttpRequestMethod(httpClientRequest);
}

@Override
public List<String> getHttpRequestHeader(HttpClientRequest httpClientRequest, String name) {
DeprecatedClassLogger.logWarning(getClass());
if (httpClientRequest == null) {
return Collections.emptyList();
}
Expand All @@ -116,13 +128,15 @@ public List<String> getHttpRequestHeader(HttpClientRequest httpClientRequest, St
*/
@Deprecated
public List<String> getRequestHeader(HttpClientRequest httpClientRequest, String name) {
DeprecatedClassLogger.logWarning(getClass());
return this.getHttpRequestHeader(httpClientRequest, name);
}

@Nullable
@Override
public Integer getHttpResponseStatusCode(HttpClientRequest httpClientRequest, HttpClientResponse httpClientResponse,
@Nullable Throwable error) {
DeprecatedClassLogger.logWarning(getClass());
if (httpClientResponse == null) {
return null;
}
Expand All @@ -138,12 +152,14 @@ public Integer getHttpResponseStatusCode(HttpClientRequest httpClientRequest, Ht
@Deprecated
public Integer getStatusCode(HttpClientRequest httpClientRequest, HttpClientResponse httpClientResponse,
Throwable error) {
DeprecatedClassLogger.logWarning(getClass());
return this.getHttpResponseStatusCode(httpClientRequest, httpClientResponse, error);
}

@Override
public List<String> getHttpResponseHeader(HttpClientRequest httpClientRequest,
HttpClientResponse httpClientResponse, String name) {
DeprecatedClassLogger.logWarning(getClass());
if (httpClientResponse == null) {
return Collections.emptyList();
}
Expand All @@ -164,6 +180,7 @@ public List<String> getHttpResponseHeader(HttpClientRequest httpClientRequest,
@Deprecated
public List<String> getResponseHeader(HttpClientRequest httpClientRequest, HttpClientResponse httpClientResponse,
String name) {
DeprecatedClassLogger.logWarning(getClass());
return this.getHttpResponseHeader(httpClientRequest, httpClientResponse, name);
}

Expand Down
Loading

0 comments on commit fce71d2

Please sign in to comment.