Skip to content

Db error type #13640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 10, 2025
Merged

Db error type #13640

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
81f3150
extract method
zeitlinger Apr 2, 2025
5dc4f51
cleanup test code
zeitlinger Apr 2, 2025
aeb5b6b
support newer Database ex
zeitlinger Apr 2, 2025
f89d54b
support newer Database ex
zeitlinger Apr 2, 2025
507442b
pr review
zeitlinger Apr 2, 2025
8ab9374
test functionality is covered by latest dep tests
zeitlinger Apr 3, 2025
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
24 changes: 24 additions & 0 deletions .../java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbResponseStatusUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.incubator.semconv.db;

import javax.annotation.Nullable;

public class DbResponseStatusUtil {
Copy link
Contributor

@laurit laurit Apr 2, 2025

Choose a reason for hiding this comment

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

should be final, also I think it should be moved to internal package

private DbResponseStatusUtil() {}

@Nullable
public static String dbResponseStatusCode(int responseStatusCode) {
return isError(responseStatusCode) ? Integer.toString(responseStatusCode) : null;
}

private static boolean isError(int responseStatusCode) {
return responseStatusCode >= 400
||
// invalid status code, does not exist
responseStatusCode < 100;
}
}
10 changes: 8 additions & 2 deletions ...ion-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
*/
public final class SemconvStability {

private static final boolean emitOldDatabaseSemconv;
private static final boolean emitStableDatabaseSemconv;
private static boolean emitOldDatabaseSemconv;
private static boolean emitStableDatabaseSemconv;

static {
boolean oldDatabase = true;
Expand Down Expand Up @@ -78,4 +78,10 @@ public static String stableDbSystemName(String oldDbSystem) {
}

private SemconvStability() {}

public static void setForTesting(
boolean emitOldDatabaseSemconv, boolean emitStableDatabaseSemconv) {
SemconvStability.emitOldDatabaseSemconv = emitOldDatabaseSemconv;
SemconvStability.emitStableDatabaseSemconv = emitStableDatabaseSemconv;
}
}
4 changes: 2 additions & 2 deletions .../main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpStatusCodeConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ enum HttpStatusCodeConverter {
boolean isError(int responseStatusCode) {
return responseStatusCode >= 500
||
// invalid status code, does not exists
// invalid status code, does not exist
responseStatusCode < 100;
}
},
Expand All @@ -21,7 +21,7 @@ boolean isError(int responseStatusCode) {
boolean isError(int responseStatusCode) {
return responseStatusCode >= 400
||
// invalid status code, does not exists
// invalid status code, does not exist
responseStatusCode < 100;
}
};
Expand Down
7 changes: 2 additions & 5 deletions ...trumentation/elasticsearch/rest/common/v5_0/internal/ElasticsearchDbAttributesGetter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package io.opentelemetry.instrumentation.elasticsearch.rest.common.v5_0.internal;

import static io.opentelemetry.instrumentation.api.incubator.semconv.db.DbResponseStatusUtil.dbResponseStatusCode;
import static java.util.logging.Level.FINE;

import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientAttributesGetter;
Expand Down Expand Up @@ -96,10 +97,6 @@ public String getDbOperationName(ElasticsearchRestRequest request) {
@Nullable
@Override
public String getResponseStatus(@Nullable Response response, @Nullable Throwable error) {
if (response != null) {
int httpStatus = response.getStatusLine().getStatusCode();
return httpStatus >= 400 && httpStatus < 600 ? Integer.toString(httpStatus) : null;
}
return null;
return response != null ? dbResponseStatusCode(response.getStatusLine().getStatusCode()) : null;
}
}
8 changes: 3 additions & 5 deletions ...entelemetry/javaagent/instrumentation/opensearch/rest/OpenSearchRestAttributesGetter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package io.opentelemetry.javaagent.instrumentation.opensearch.rest;

import static io.opentelemetry.instrumentation.api.incubator.semconv.db.DbResponseStatusUtil.dbResponseStatusCode;

import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientAttributesGetter;
import io.opentelemetry.semconv.incubating.DbIncubatingAttributes;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -54,10 +56,6 @@ public String getDbOperationName(OpenSearchRestRequest request) {
@Nullable
@Override
public String getResponseStatus(@Nullable Response response, @Nullable Throwable error) {
if (response != null) {
int httpStatus = response.getStatusLine().getStatusCode();
return httpStatus >= 400 && httpStatus < 600 ? Integer.toString(httpStatus) : null;
}
return null;
return response != null ? dbResponseStatusCode(response.getStatusLine().getStatusCode()) : null;
}
}
58 changes: 27 additions & 31 deletions ...c/test/java/io/opentelemetry/javaagent/instrumentation/spymemcached/SpymemcachedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,14 @@
import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.sdk.testing.assertj.AttributeAssertion;
import io.opentelemetry.sdk.trace.data.StatusData;
import io.opentelemetry.semconv.incubating.DbIncubatingAttributes;
import java.net.InetSocketAddress;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue;
Expand Down Expand Up @@ -263,22 +260,15 @@ void getTimeout() throws InterruptedException {
EXCEPTION_STACKTRACE,
val -> val.isInstanceOf(String.class))))
.hasAttributesSatisfyingExactly(
withErrorType(
"net.spy.memcached.internal.CheckedOperationTimeoutException",
equalTo(
maybeStable(DB_SYSTEM),
DbIncubatingAttributes.DbSystemIncubatingValues.MEMCACHED),
equalTo(maybeStable(DB_OPERATION), "get")))));
}

private static List<AttributeAssertion> withErrorType(
String errorType, AttributeAssertion... assertions) {
List<AttributeAssertion> list = new ArrayList<>(Arrays.asList(assertions));

if (SemconvStability.emitStableDatabaseSemconv()) {
list.add(equalTo(ERROR_TYPE, errorType));
}
return list;
equalTo(
ERROR_TYPE,
SemconvStability.emitStableDatabaseSemconv()
? "net.spy.memcached.internal.CheckedOperationTimeoutException"
: null),
equalTo(
maybeStable(DB_SYSTEM),
DbIncubatingAttributes.DbSystemIncubatingValues.MEMCACHED),
equalTo(maybeStable(DB_OPERATION), "get"))));
}

@Test
Expand Down Expand Up @@ -896,12 +886,15 @@ void decrException() {
.hasException(
new IllegalArgumentException("Key is too long (maxlen = 250)"))
.hasAttributesSatisfyingExactly(
withErrorType(
"java.lang.IllegalArgumentException",
equalTo(
maybeStable(DB_SYSTEM),
DbIncubatingAttributes.DbSystemIncubatingValues.MEMCACHED),
equalTo(maybeStable(DB_OPERATION), "decr")))));
equalTo(
ERROR_TYPE,
SemconvStability.emitStableDatabaseSemconv()
? "java.lang.IllegalArgumentException"
: null),
equalTo(
maybeStable(DB_SYSTEM),
DbIncubatingAttributes.DbSystemIncubatingValues.MEMCACHED),
equalTo(maybeStable(DB_OPERATION), "decr"))));
}

@Test
Expand Down Expand Up @@ -984,12 +977,15 @@ void incrException() {
.hasException(
new IllegalArgumentException("Key is too long (maxlen = 250)"))
.hasAttributesSatisfyingExactly(
withErrorType(
"java.lang.IllegalArgumentException",
equalTo(
maybeStable(DB_SYSTEM),
DbIncubatingAttributes.DbSystemIncubatingValues.MEMCACHED),
equalTo(maybeStable(DB_OPERATION), "incr")))));
equalTo(
ERROR_TYPE,
SemconvStability.emitStableDatabaseSemconv()
? "java.lang.IllegalArgumentException"
: null),
equalTo(
maybeStable(DB_SYSTEM),
DbIncubatingAttributes.DbSystemIncubatingValues.MEMCACHED),
equalTo(maybeStable(DB_OPERATION), "incr"))));
}

private static String key(String k) {
Expand Down
26 changes: 22 additions & 4 deletions instrumentation/vertx/vertx-sql-client-4.0/javaagent/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,31 @@ muzzle {
}

dependencies {
library("io.vertx:vertx-sql-client:4.0.0")
compileOnly("io.vertx:vertx-codegen:4.0.0")
val version = "4.0.0"
library("io.vertx:vertx-sql-client:$version")
compileOnly("io.vertx:vertx-codegen:$version")

testInstrumentation(project(":instrumentation:netty:netty-4.1:javaagent"))

testLibrary("io.vertx:vertx-pg-client:4.0.0")
testLibrary("io.vertx:vertx-codegen:4.0.0")
testLibrary("io.vertx:vertx-pg-client:$version")
testLibrary("io.vertx:vertx-codegen:$version")
}

testing {
Copy link
Contributor

@laurit laurit Apr 3, 2025

Choose a reason for hiding this comment

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

Why do you need this suite? Try running ./gradlew :instrumentation:vertx:vertx-sql-client-4.0:javaagent:build -PtestLatestDeps=true doesn't it already run the testStableSemconv task with the latest dependency?

Copy link
Member Author

@zeitlinger zeitlinger Apr 3, 2025

Choose a reason for hiding this comment

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

good point - I had originally thought that the test would need to be different - but it turned out that it doesn't

Copy link
Member Author

@zeitlinger zeitlinger Apr 3, 2025

Choose a reason for hiding this comment

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

I was also not sure about the extend we should rely on latest dep tests for things like that

suites {
val testVertx5 by registering(JvmTestSuite::class) {
val version = "4.5.13"
dependencies {
implementation(project())
implementation("io.vertx:vertx-sql-client:$version")
implementation("io.vertx:vertx-codegen:$version")
implementation("io.vertx:vertx-pg-client:$version")
implementation("org.testcontainers:testcontainers")

implementation(project(":instrumentation:netty:netty-4.1:javaagent"))
}
}
}
}

tasks {
Expand Down
50 changes: 40 additions & 10 deletions ...pentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientAttributesGetter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,20 @@

import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlClientAttributesGetter;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.function.Function;
import javax.annotation.Nullable;

public enum VertxSqlClientAttributesGetter
implements SqlClientAttributesGetter<VertxSqlClientRequest, Void> {
INSTANCE;

private static final List<Function<Exception, String>> responseStatusExtractors =
createResponseStatusExtractors();

@Override
public String getDbSystem(VertxSqlClientRequest request) {
return null;
Expand Down Expand Up @@ -49,18 +56,41 @@ public Collection<String> getRawQueryTexts(VertxSqlClientRequest request) {
@Nullable
@Override
public String getResponseStatus(@Nullable Void response, @Nullable Throwable error) {
try {
// loaded via reflection, because this class is not available in all versions that we support
Class<?> ex = Class.forName("io.vertx.pgclient.PgException");
if (ex.isInstance(error)) {
return (String) ex.getMethod("getCode").invoke(error);
for (Function<Exception, String> extractor : responseStatusExtractors) {
String status = extractor.apply((Exception) error);
if (status != null) {
return status;
}
} catch (ClassNotFoundException
| NoSuchMethodException
| IllegalAccessException
| InvocationTargetException e) {
return null;
}
return null;
}

private static List<Function<Exception, String>> createResponseStatusExtractors() {
return Arrays.asList(
responseStatusExtractor("io.vertx.sqlclient.DatabaseException", "getSqlState"),
// older version only have this method
responseStatusExtractor("io.vertx.pgclient.PgException", "getCode"));
}

private static Function<Exception, String> responseStatusExtractor(
String className, String methodName) {
try {
// loaded via reflection, because this class is not available in all versions that we support
Class<?> exClass = Class.forName(className);
Method method = exClass.getDeclaredMethod(methodName);

return (error) -> {
if (exClass.isInstance(error)) {
try {
return String.valueOf(method.invoke(error)); // can be String or int
} catch (IllegalAccessException | InvocationTargetException e) {
return null;
}
}
return null;
};
} catch (ClassNotFoundException | NoSuchMethodException e) {
return (error) -> null;
}
}
}
Loading
Loading