Skip to content

Commit

Permalink
Revert "Remove db.connection_string (#11089)" (#11366)
Browse files Browse the repository at this point in the history
  • Loading branch information
trask authored May 16, 2024
1 parent 66e4c5a commit 68ebe00
Show file tree
Hide file tree
Showing 55 changed files with 512 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ abstract class DbClientCommonAttributesExtractor<
private static final AttributeKey<String> DB_NAME = AttributeKey.stringKey("db.name");
private static final AttributeKey<String> DB_SYSTEM = AttributeKey.stringKey("db.system");
private static final AttributeKey<String> DB_USER = AttributeKey.stringKey("db.user");
private static final AttributeKey<String> DB_CONNECTION_STRING =
AttributeKey.stringKey("db.connection_string");

final GETTER getter;

Expand All @@ -35,6 +37,7 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
internalSet(attributes, DB_SYSTEM, getter.getSystem(request));
internalSet(attributes, DB_USER, getter.getUser(request));
internalSet(attributes, DB_NAME, getter.getName(request));
internalSet(attributes, DB_CONNECTION_STRING, getter.getConnectionString(request));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ public interface DbClientCommonAttributesGetter<REQUEST> {
@Nullable
String getName(REQUEST request);

// to be removed in 2.4.0, use `server.address` and `server.port` instead
@Nullable
@Deprecated
default String getConnectionString(REQUEST request) {
return null;
}
String getConnectionString(REQUEST request);
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ public String getName(Map<String, String> map) {
return map.get("db.name");
}

@Override
public String getConnectionString(Map<String, String> map) {
return map.get("db.connection_string");
}

@Override
public String getStatement(Map<String, String> map) {
return map.get("db.statement");
Expand All @@ -47,13 +52,15 @@ public String getOperation(Map<String, String> map) {
}
}

@SuppressWarnings("deprecation") // TODO DbIncubatingAttributes.DB_CONNECTION_STRING deprecation
@Test
void shouldExtractAllAvailableAttributes() {
// given
Map<String, String> request = new HashMap<>();
request.put("db.system", "myDb");
request.put("db.user", "username");
request.put("db.name", "potatoes");
request.put("db.connection_string", "mydb:///potatoes");
request.put("db.statement", "SELECT * FROM potato");
request.put("db.operation", "SELECT");

Expand All @@ -75,6 +82,7 @@ void shouldExtractAllAvailableAttributes() {
entry(DbIncubatingAttributes.DB_SYSTEM, "myDb"),
entry(DbIncubatingAttributes.DB_USER, "username"),
entry(DbIncubatingAttributes.DB_NAME, "potatoes"),
entry(DbIncubatingAttributes.DB_CONNECTION_STRING, "mydb:///potatoes"),
entry(DbIncubatingAttributes.DB_STATEMENT, "SELECT * FROM potato"),
entry(DbIncubatingAttributes.DB_OPERATION, "SELECT"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,22 @@ public String getUser(Map<String, String> map) {
public String getName(Map<String, String> map) {
return map.get("db.name");
}

@Override
public String getConnectionString(Map<String, String> map) {
return map.get("db.connection_string");
}
}

@SuppressWarnings("deprecation") // TODO DbIncubatingAttributes.DB_CONNECTION_STRING deprecation
@Test
void shouldExtractAllAttributes() {
// given
Map<String, String> request = new HashMap<>();
request.put("db.system", "myDb");
request.put("db.user", "username");
request.put("db.name", "potatoes");
request.put("db.connection_string", "mydb:///potatoes");
request.put("db.statement", "SELECT * FROM potato WHERE id=12345");

Context context = Context.root();
Expand All @@ -71,6 +78,7 @@ void shouldExtractAllAttributes() {
entry(DbIncubatingAttributes.DB_SYSTEM, "myDb"),
entry(DbIncubatingAttributes.DB_USER, "username"),
entry(DbIncubatingAttributes.DB_NAME, "potatoes"),
entry(DbIncubatingAttributes.DB_CONNECTION_STRING, "mydb:///potatoes"),
entry(DbIncubatingAttributes.DB_STATEMENT, "SELECT * FROM potato WHERE id=?"),
entry(DbIncubatingAttributes.DB_OPERATION, "SELECT"),
entry(DbIncubatingAttributes.DB_SQL_TABLE, "potato"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ public String getName(CassandraRequest request) {
return request.getSession().getLoggedKeyspace();
}

@Override
@Nullable
public String getConnectionString(CassandraRequest request) {
return null;
}

@Override
@Nullable
public String getRawStatement(CassandraRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ public String getName(CassandraRequest request) {
return request.getSession().getKeyspace().map(CqlIdentifier::toString).orElse(null);
}

@Override
@Nullable
public String getConnectionString(CassandraRequest request) {
return null;
}

@Override
@Nullable
public String getRawStatement(CassandraRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ public String getName(CassandraRequest request) {
return request.getSession().getKeyspace().map(CqlIdentifier::toString).orElse(null);
}

@Override
@Nullable
public String getConnectionString(CassandraRequest request) {
return null;
}

@Override
@Nullable
public String getRawStatement(CassandraRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ public String getName(CouchbaseRequestInfo couchbaseRequest) {
return couchbaseRequest.bucket();
}

@Override
@Nullable
public String getConnectionString(CouchbaseRequestInfo couchbaseRequest) {
return null;
}

@Override
@Nullable
public String getStatement(CouchbaseRequestInfo couchbaseRequest) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ public String getName(ElasticsearchRestRequest request) {
return null;
}

@Override
@Nullable
public String getConnectionString(ElasticsearchRestRequest request) {
return null;
}

@Override
@Nullable
public String getStatement(ElasticsearchRestRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ public String getName(ElasticTransportRequest s) {
return null;
}

@Override
@Nullable
public String getConnectionString(ElasticTransportRequest s) {
return null;
}

@Override
@Nullable
public String getStatement(ElasticTransportRequest s) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ public String getName(GeodeRequest request) {
return request.getRegion().getName();
}

@Override
@Nullable
public String getConnectionString(GeodeRequest request) {
return null;
}

@Override
@Nullable
public String getStatement(GeodeRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,21 @@ static void cleanUp() {
}
}

@SuppressWarnings("deprecation") // TODO DbIncubatingAttributes.DB_CONNECTION_STRING deprecation
static SpanDataAssert assertClientSpan(SpanDataAssert span, SpanData parent) {
return span.hasKind(SpanKind.CLIENT)
.hasParent(parent)
.hasAttributesSatisfyingExactly(
equalTo(DbIncubatingAttributes.DB_SYSTEM, "h2"),
equalTo(DbIncubatingAttributes.DB_NAME, "db1"),
equalTo(DbIncubatingAttributes.DB_USER, "sa"),
equalTo(DbIncubatingAttributes.DB_CONNECTION_STRING, "h2:mem:"),
satisfies(DbIncubatingAttributes.DB_STATEMENT, val -> val.isInstanceOf(String.class)),
satisfies(DbIncubatingAttributes.DB_OPERATION, val -> val.isInstanceOf(String.class)),
equalTo(DbIncubatingAttributes.DB_SQL_TABLE, "Value"));
}

@SuppressWarnings("deprecation") // TODO DbIncubatingAttributes.DB_CONNECTION_STRING deprecation
static SpanDataAssert assertClientSpan(SpanDataAssert span, SpanData parent, String verb) {
return span.hasName(verb.concat(" db1.Value"))
.hasKind(SpanKind.CLIENT)
Expand All @@ -77,6 +80,7 @@ static SpanDataAssert assertClientSpan(SpanDataAssert span, SpanData parent, Str
equalTo(DbIncubatingAttributes.DB_SYSTEM, "h2"),
equalTo(DbIncubatingAttributes.DB_NAME, "db1"),
equalTo(DbIncubatingAttributes.DB_USER, "sa"),
equalTo(DbIncubatingAttributes.DB_CONNECTION_STRING, "h2:mem:"),
satisfies(
DbIncubatingAttributes.DB_STATEMENT,
stringAssert -> stringAssert.startsWith(verb.toLowerCase(Locale.ROOT))),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ void testCriteria(String methodName, Consumer<Criteria> interaction) {
equalTo(DbIncubatingAttributes.DB_SYSTEM, "h2"),
equalTo(DbIncubatingAttributes.DB_NAME, "db1"),
equalTo(DbIncubatingAttributes.DB_USER, "sa"),
equalTo(DbIncubatingAttributes.DB_CONNECTION_STRING, "h2:mem:"),
satisfies(
DbIncubatingAttributes.DB_STATEMENT,
stringAssert -> stringAssert.startsWith("select")),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class EntityManagerTest extends AbstractHibernateTest {
static final EntityManagerFactory entityManagerFactory =
Persistence.createEntityManagerFactory("test-pu");

@SuppressWarnings("deprecation") // TODO DbIncubatingAttributes.DB_CONNECTION_STRING deprecation
@ParameterizedTest
@MethodSource("provideArgumentsHibernateActionParameters")
void testHibernateActions(Parameter parameter) {
Expand Down Expand Up @@ -102,6 +103,7 @@ void testHibernateActions(Parameter parameter) {
equalTo(DbIncubatingAttributes.DB_SYSTEM, "h2"),
equalTo(DbIncubatingAttributes.DB_NAME, "db1"),
equalTo(DbIncubatingAttributes.DB_USER, "sa"),
equalTo(DbIncubatingAttributes.DB_CONNECTION_STRING, "h2:mem:"),
satisfies(
DbIncubatingAttributes.DB_STATEMENT,
val -> val.isInstanceOf(String.class)),
Expand Down Expand Up @@ -132,6 +134,7 @@ void testHibernateActions(Parameter parameter) {
equalTo(DbIncubatingAttributes.DB_SYSTEM, "h2"),
equalTo(DbIncubatingAttributes.DB_NAME, "db1"),
equalTo(DbIncubatingAttributes.DB_USER, "sa"),
equalTo(DbIncubatingAttributes.DB_CONNECTION_STRING, "h2:mem:"),
satisfies(
DbIncubatingAttributes.DB_STATEMENT,
val -> val.isInstanceOf(String.class)),
Expand Down Expand Up @@ -181,6 +184,7 @@ private static Stream<Arguments> provideArgumentsHibernateActionParameters() {
Arguments.of(named("remove", new Parameter("delete", true, true, EntityManager::remove))));
}

@SuppressWarnings("deprecation") // TODO DbIncubatingAttributes.DB_CONNECTION_STRING deprecation
@Test
void testHibernatePersist() {
EntityManager entityManager = entityManagerFactory.createEntityManager();
Expand Down Expand Up @@ -216,6 +220,7 @@ void testHibernatePersist() {
equalTo(DbIncubatingAttributes.DB_SYSTEM, "h2"),
equalTo(DbIncubatingAttributes.DB_NAME, "db1"),
equalTo(DbIncubatingAttributes.DB_USER, "sa"),
equalTo(DbIncubatingAttributes.DB_CONNECTION_STRING, "h2:mem:"),
satisfies(
DbIncubatingAttributes.DB_STATEMENT,
val -> val.isInstanceOf(String.class)),
Expand All @@ -241,6 +246,7 @@ void testHibernatePersist() {
equalTo(DbIncubatingAttributes.DB_SYSTEM, "h2"),
equalTo(DbIncubatingAttributes.DB_NAME, "db1"),
equalTo(DbIncubatingAttributes.DB_USER, "sa"),
equalTo(DbIncubatingAttributes.DB_CONNECTION_STRING, "h2:mem:"),
satisfies(
DbIncubatingAttributes.DB_STATEMENT,
val -> val.isInstanceOf(String.class)),
Expand All @@ -250,6 +256,7 @@ void testHibernatePersist() {
equalTo(DbIncubatingAttributes.DB_SQL_TABLE, "Value"))));
}

@SuppressWarnings("deprecation") // TODO DbIncubatingAttributes.DB_CONNECTION_STRING deprecation
@ParameterizedTest
@MethodSource("provideArgumentsAttachesState")
void testAttachesStateToQuery(Function<EntityManager, Query> queryBuildMethod) {
Expand Down Expand Up @@ -288,6 +295,7 @@ void testAttachesStateToQuery(Function<EntityManager, Query> queryBuildMethod) {
equalTo(DbIncubatingAttributes.DB_SYSTEM, "h2"),
equalTo(DbIncubatingAttributes.DB_NAME, "db1"),
equalTo(DbIncubatingAttributes.DB_USER, "sa"),
equalTo(DbIncubatingAttributes.DB_CONNECTION_STRING, "h2:mem:"),
satisfies(
DbIncubatingAttributes.DB_STATEMENT,
val -> val.isInstanceOf(String.class)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

class QueryTest extends AbstractHibernateTest {

@SuppressWarnings("deprecation") // TODO DbIncubatingAttributes.DB_CONNECTION_STRING deprecation
@Test
void testHibernateQueryExecuteUpdateWithTransaction() {
testing.runWithSpan(
Expand Down Expand Up @@ -63,6 +64,7 @@ void testHibernateQueryExecuteUpdateWithTransaction() {
equalTo(DbIncubatingAttributes.DB_SYSTEM, "h2"),
equalTo(DbIncubatingAttributes.DB_NAME, "db1"),
equalTo(DbIncubatingAttributes.DB_USER, "sa"),
equalTo(DbIncubatingAttributes.DB_CONNECTION_STRING, "h2:mem:"),
satisfies(
DbIncubatingAttributes.DB_STATEMENT,
val -> val.isInstanceOf(String.class)),
Expand All @@ -83,6 +85,7 @@ void testHibernateQueryExecuteUpdateWithTransaction() {
.get(stringKey("hibernate.session_id"))))));
}

@SuppressWarnings("deprecation") // TODO DbIncubatingAttributes.DB_CONNECTION_STRING deprecation
@ParameterizedTest
@MethodSource("providesArgumentsSingleCall")
void testHibernateQuerySingleCall(Parameter parameter) {
Expand Down Expand Up @@ -119,6 +122,7 @@ void testHibernateQuerySingleCall(Parameter parameter) {
equalTo(DbIncubatingAttributes.DB_SYSTEM, "h2"),
equalTo(DbIncubatingAttributes.DB_NAME, "db1"),
equalTo(DbIncubatingAttributes.DB_USER, "sa"),
equalTo(DbIncubatingAttributes.DB_CONNECTION_STRING, "h2:mem:"),
satisfies(
DbIncubatingAttributes.DB_STATEMENT,
val -> val.startsWith("select ")),
Expand Down Expand Up @@ -151,6 +155,7 @@ private static Stream<Arguments> providesArgumentsSingleCall() {
new Parameter("SELECT Value", sess -> sess.createQuery("from Value").scroll()))));
}

@SuppressWarnings("deprecation") // TODO DbIncubatingAttributes.DB_CONNECTION_STRING deprecation
@Test
void testHibernateQueryIterate() {
testing.runWithSpan(
Expand Down Expand Up @@ -191,6 +196,7 @@ void testHibernateQueryIterate() {
equalTo(DbIncubatingAttributes.DB_SYSTEM, "h2"),
equalTo(DbIncubatingAttributes.DB_NAME, "db1"),
equalTo(DbIncubatingAttributes.DB_USER, "sa"),
equalTo(DbIncubatingAttributes.DB_CONNECTION_STRING, "h2:mem:"),
satisfies(
DbIncubatingAttributes.DB_STATEMENT,
val -> val.startsWith("select ")),
Expand Down
Loading

0 comments on commit 68ebe00

Please sign in to comment.