Skip to content

Commit 64ec587

Browse files
PastranaPastrana
authored andcommitted
HPCC4J-612 Code review
- Removes commented out lines - Adds WithSpan annotations to wsclientpool - Adds hascredentials span attribute to current span - Ensures executeECLScript is traced - Externalizes opentelemetry-instrumentation-annotations version variable Signed-off-by: Pastrana <[email protected]>
1 parent f68e38b commit 64ec587

File tree

5 files changed

+37
-46
lines changed

5 files changed

+37
-46
lines changed

pom.xml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
<project.benchmarking>false</project.benchmarking>
6565
<opentelemetry.bom.version>1.38.0</opentelemetry.bom.version>
6666
<opentelemetry.semconv.version>1.25.0-alpha</opentelemetry.semconv.version>
67+
<opentelemetry.instruopentelemetry.instrumentation.annotations.version>2.6.0</opentelemetry.instruopentelemetry.instrumentation.annotations.version>
6768
</properties>
6869

6970
<scm>
@@ -139,7 +140,7 @@
139140
<!-- Not managed by opentelemetry-bom -->
140141
<groupId>io.opentelemetry.instrumentation</groupId>
141142
<artifactId>opentelemetry-instrumentation-annotations</artifactId>
142-
<version>2.6.0</version>
143+
<version>${opentelemetry.instruopentelemetry.instrumentation.annotations.version}</version>
143144
</dependency>
144145
<dependency>
145146
<!-- Not managed by opentelemetry-bom -->

wsclient/src/main/java/org/hpccsystems/ws/client/HPCCWsClientPool.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import org.hpccsystems.ws.client.utils.Connection;
44
import org.hpccsystems.ws.client.utils.ObjectPool;
55

6-
//import io.opentelemetry.instrumentation.annotations.WithSpan;
6+
import io.opentelemetry.instrumentation.annotations.WithSpan;
77

88
/**
99
* <p>HPCCWsClientPool class.</p>
@@ -78,7 +78,7 @@ public HPCCWsClientPool(Connection connection)
7878
* @param timeout
7979
* the timeout
8080
*/
81-
//@WithSpan
81+
@WithSpan
8282
public HPCCWsClientPool(Connection connection, long timeout)
8383
{
8484
super(timeout);
@@ -92,7 +92,7 @@ public HPCCWsClientPool(Connection connection, long timeout)
9292
*/
9393
/** {@inheritDoc} */
9494
@Override
95-
//@WithSpan
95+
@WithSpan
9696
protected HPCCWsClient create()
9797
{
9898
return (new HPCCWsClient(m_connection));
@@ -105,6 +105,7 @@ protected HPCCWsClient create()
105105
*/
106106
/** {@inheritDoc} */
107107
@Override
108+
@WithSpan
108109
public boolean validate(HPCCWsClient client)
109110
{
110111
try

wsclient/src/main/java/org/hpccsystems/ws/client/HPCCWsStoreClient.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@
4242
import io.opentelemetry.instrumentation.annotations.SpanAttribute;
4343
import io.opentelemetry.instrumentation.annotations.WithSpan;
4444

45-
//import io.opentelemetry.instrumentation.annotations.WithSpan;
46-
4745
/**
4846
* Facilitates access to HPCC Systems key/value based Storage.
4947
*

wsclient/src/main/java/org/hpccsystems/ws/client/utils/Connection.java

Lines changed: 29 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,61 +1065,50 @@ public String sendHTTPRequest(@SpanAttribute String uri, @SpanAttribute String m
10651065

10661066
URL url = new URL (getBaseUrl() + (uri != null && uri.startsWith("/") ? "" : "/") + uri);
10671067

1068-
/*Span sendHTTPReqSpan = GlobalOpenTelemetry.get().getTracer(BaseHPCCWsClient.PROJECT_NAME)
1069-
.spanBuilder(method.toUpperCase() + " " + url.toExternalForm())
1070-
.setAttribute(ServerAttributes.SERVER_ADDRESS, getHost())
1071-
.setAttribute(ServerAttributes.SERVER_PORT, Long.getLong(getPort()))
1072-
.setAttribute(HttpAttributes.HTTP_REQUEST_METHOD, method)
1073-
.setSpanKind(SpanKind.CLIENT)
1074-
.startSpan();
1075-
*/
10761068
HttpURLConnection httpURLConnection = (HttpURLConnection) url.openConnection(); //throws IOException
10771069

10781070
Connection.log.info("Sending HTTP " + method + "Request to:" + url.toString());
10791071

1072+
boolean isTraced = Span.current().isRecording();
10801073
if (hasCredentials())
10811074
{
10821075
httpURLConnection.setRequestProperty("Authorization", getBasicAuthString());
1083-
//sendHTTPReqSpan.setAttribute("hasCredentials", true);
1076+
if (isTraced)
1077+
Span.current().setAttribute("hasCredentials", true);
10841078
}
1085-
//else
1086-
//{
1087-
//sendHTTPReqSpan.setAttribute("hasCredentials", false);
1088-
//}
1089-
1090-
//try (Scope scope = sendHTTPReqSpan.makeCurrent())
1079+
else
10911080
{
1092-
httpURLConnection.setRequestProperty("traceparent", Utils.getCurrentSpanTraceParentHeader());
1093-
httpURLConnection.setRequestMethod(method); //throws ProtocolException
1081+
if (isTraced)
1082+
Span.current().setAttribute("hasCredentials", false);
1083+
}
10941084

1095-
int responseCode = httpURLConnection.getResponseCode(); //throws IOException
1096-
//sendHTTPReqSpan.setAttribute("http.response.status_code", responseCode);
1097-
Connection.log.info("HTTP Response code: " + responseCode);
1085+
if (isTraced)
1086+
httpURLConnection.setRequestProperty("traceparent", Utils.getCurrentSpanTraceParentHeader());
10981087

1099-
if (responseCode == HttpURLConnection.HTTP_OK) //success
1100-
{
1101-
BufferedReader in = new BufferedReader(new InputStreamReader(httpURLConnection.getInputStream())); //throws IOException
1102-
String inputLine;
1103-
StringBuffer response = new StringBuffer();
1088+
httpURLConnection.setRequestMethod(method); //throws ProtocolException
11041089

1105-
while ((inputLine = in.readLine()) != null) // throws IOException
1106-
{
1107-
response.append(inputLine);
1108-
}
1090+
int responseCode = httpURLConnection.getResponseCode(); //throws IOException
11091091

1110-
in.close(); //throws IOException
1111-
// sendHTTPReqSpan.setStatus(StatusCode.OK);
1112-
return response.toString();
1113-
}
1114-
else
1092+
Connection.log.info("HTTP Response code: " + responseCode);
1093+
1094+
if (responseCode == HttpURLConnection.HTTP_OK) //success
1095+
{
1096+
BufferedReader in = new BufferedReader(new InputStreamReader(httpURLConnection.getInputStream())); //throws IOException
1097+
String inputLine;
1098+
StringBuffer response = new StringBuffer();
1099+
1100+
while ((inputLine = in.readLine()) != null) // throws IOException
11151101
{
1116-
//sendHTTPReqSpan.setStatus(StatusCode.ERROR);
1117-
throw new IOException("HTTP request failed! Code (" + responseCode + ") " + httpURLConnection.getResponseMessage() );
1102+
response.append(inputLine);
11181103
}
1104+
1105+
in.close(); //throws IOException
1106+
1107+
return response.toString();
1108+
}
1109+
else
1110+
{
1111+
throw new IOException("HTTP request failed! Code (" + responseCode + ") " + httpURLConnection.getResponseMessage() );
11191112
}
1120-
//finally
1121-
//{
1122-
// sendHTTPReqSpan.end();
1123-
//}
11241113
}
11251114
}

wsclient/src/test/java/org/hpccsystems/ws/client/BaseRemoteTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ HPCC SYSTEMS software Copyright (C) 2019 HPCC Systems®.
4545
import io.opentelemetry.api.OpenTelemetry;
4646
import io.opentelemetry.api.trace.SpanBuilder;
4747
import io.opentelemetry.api.trace.Tracer;
48+
import io.opentelemetry.instrumentation.annotations.WithSpan;
4849
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
4950

5051
@Category(org.hpccsystems.commons.annotations.RemoteTests.class)
@@ -285,6 +286,7 @@ public boolean verify(String hostname,javax.net.ssl.SSLSession sslSession)
285286
}
286287
}
287288

289+
@WithSpan
288290
public static String executeECLScript(String eclFile) throws Exception
289291
{
290292
InputStream resourceStream = BaseRemoteTest.class.getClassLoader().getResourceAsStream(eclFile);

0 commit comments

Comments
 (0)