From a837d43aa3fa189c19ae5dadaa4e0e28ae52d97a Mon Sep 17 00:00:00 2001 From: SylvainJuge Date: Wed, 2 Jun 2021 16:59:03 +0200 Subject: [PATCH] Shared JSON spec for span types/subtypes (#1812) * first draft for JSON spec Co-authored-by: eyalkoren <41850454+eyalkoren@users.noreply.github.com> Co-authored-by: eyalkoren <41850454+eyalkoren@users.noreply.github.com> --- .../apm/agent/impl/transaction/Span.java | 17 ++- .../report/serialize/DslJsonSerializer.java | 6 +- .../agent/AbstractInstrumentationTest.java | 3 + .../co/elastic/apm/agent/MockReporter.java | 131 ++++++++++++++--- .../apm/agent/impl/SpanTypeBreakdownTest.java | 13 +- .../apm/agent/impl/transaction/SpanTest.java | 11 +- .../test/resources/json-specs/span_types.json | 138 ++++++++++++++++++ .../pluginapi/CaptureSpanInstrumentation.java | 6 +- .../pluginapi/SpanInstrumentationTest.java | 7 + .../TransactionInstrumentationTest.java | 9 +- .../co/elastic/apm/api/AnnotationApiTest.java | 21 ++- .../BlockingQueueContextPropagationTest.java | 4 + .../api/ElasticApmApiInstrumentationTest.java | 5 +- ...sticsearchRestClientInstrumentationIT.java | 18 +-- .../AbstractEsClientInstrumentationTest.java | 28 ++-- .../HibernateSearch5InstrumentationTest.java | 2 + .../HibernateSearch6InstrumentationTest.java | 3 + .../apm/agent/jdbc/helper/JdbcHelper.java | 10 +- .../jdbc/AbstractJdbcInstrumentationTest.java | 6 +- .../jdbc/helper/ConnectionMetaDataTest.java | 40 ++++- .../apm/agent/kafka/KafkaLegacyClientIT.java | 2 +- .../co/elastic/apm/agent/kafka/KafkaIT.java | 3 +- .../apm/agent/kafka/KafkaLegacyBrokerIT.java | 4 +- .../ApmSpanInstrumentation.java | 26 +--- .../apm/agent/process/ProcessHelper.java | 2 - .../apm/agent/process/ProcessHelperTest.java | 6 +- .../agent/profiler/SamplingProfilerTest.java | 2 +- .../agent/rabbitmq/AbstractRabbitMqTest.java | 3 +- .../rabbitmq/config/BaseConfiguration.java | 2 +- .../lettuce/Lettuce3InstrumentationTest.java | 2 +- .../lettuce/Lettuce4InstrumentationTest.java | 2 +- .../lettuce/Lettuce5InstrumentationTest.java | 2 +- .../servlet/ServletInstrumentationTest.java | 7 + .../opentracing/OpenTracingBridgeTest.java | 13 +- .../apm/servlet/tests/ServletApiTestApp.java | 2 +- .../plugin/PluginInstrumentationTest.java | 10 +- 36 files changed, 439 insertions(+), 127 deletions(-) create mode 100644 apm-agent-core/src/test/resources/json-specs/span_types.json diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Span.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Span.java index b42d2ce5e3..240ea99ca1 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Span.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Span.java @@ -157,7 +157,7 @@ public SpanContext getContext() { * Keywords of specific relevance in the span's domain (eg: 'db', 'template', 'ext', etc) */ public Span withType(@Nullable String type) { - this.type = type; + this.type = normalizeEmpty(type); return this; } @@ -165,7 +165,7 @@ public Span withType(@Nullable String type) { * Sets the span's subtype, related to the (eg: 'mysql', 'postgresql', 'jsf' etc) */ public Span withSubtype(@Nullable String subtype) { - this.subtype = subtype; + this.subtype = normalizeEmpty(subtype); return this; } @@ -173,10 +173,15 @@ public Span withSubtype(@Nullable String subtype) { * Action related to this span (eg: 'query', 'render' etc) */ public Span withAction(@Nullable String action) { - this.action = action; + this.action = normalizeEmpty(action); return this; } + @Nullable + private static String normalizeEmpty(@Nullable String value) { + return value == null || value.isEmpty() ? null : value; + } + /** * Sets span.type, span.subtype and span.action. If no subtype and action are provided, assumes the legacy usage of hierarchical * typing system and attempts to split the type by dots to discover subtype and action. @@ -199,9 +204,9 @@ public void setType(@Nullable String type, @Nullable String subtype, @Nullable S } } } - this.type = type; - this.subtype = subtype; - this.action = action; + withType(type); + withSubtype(subtype); + withAction(action); } @Nullable diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java index 6ec213827e..154993cae3 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java @@ -705,14 +705,14 @@ private void serializeSpanType(Span span) { replace(replaceBuilder, ".", "_", 0); String subtype = span.getSubtype(); String action = span.getAction(); - if ((subtype != null && !subtype.isEmpty()) || (action != null && !action.isEmpty())) { + if (subtype != null || action != null) { replaceBuilder.append('.'); int replaceStartIndex = replaceBuilder.length() + 1; - if (subtype != null && !subtype.isEmpty()) { + if (subtype != null) { replaceBuilder.append(subtype); replace(replaceBuilder, ".", "_", replaceStartIndex); } - if (action != null && !action.isEmpty()) { + if (action != null) { replaceBuilder.append('.'); replaceStartIndex = replaceBuilder.length() + 1; replaceBuilder.append(action); diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/AbstractInstrumentationTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/AbstractInstrumentationTest.java index 32da6fe49f..3bebc79062 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/AbstractInstrumentationTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/AbstractInstrumentationTest.java @@ -89,6 +89,9 @@ public static ConfigurationRegistry getConfig() { @After @AfterEach public final void cleanUp() { + // re-enable all reporter checks + reporter.resetChecks(); + SpyConfiguration.reset(config); try { if (validateRecycling) { diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/MockReporter.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/MockReporter.java index bf8fedcaf3..27fb5ea31d 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/MockReporter.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/MockReporter.java @@ -45,6 +45,7 @@ import com.networknt.schema.ValidationMessage; import org.awaitility.core.ThrowingRunnable; import org.stagemonitor.configuration.ConfigurationRegistry; +import specs.TestJsonSpec; import java.io.IOException; import java.util.ArrayList; @@ -78,7 +79,12 @@ public class MockReporter implements Reporter { // A map of exit span type to actions that that do not support address and port discovery private static final Map> SPAN_ACTIONS_WITHOUT_ADDRESS; // And for any case the disablement of the check cannot rely on subtype (eg Redis, where Jedis supports and Lettuce does not) - private boolean disableDestinationAddressCheck; + private boolean checkDestinationAddress = true; + // Allows optional opt-out for unknown outcome + private boolean checkUnknownOutcomes = true; + // Allows optional opt-out from strick span type/sub-type checking + private boolean checkStrictSpanType = true; + private final List transactions = Collections.synchronizedList(new ArrayList<>()); private final List spans = Collections.synchronizedList(new ArrayList<>()); @@ -86,9 +92,11 @@ public class MockReporter implements Reporter { private final List bytes = new CopyOnWriteArrayList<>(); private final ObjectMapper objectMapper; private final boolean verifyJsonSchema; - private boolean checkUnknownOutcomes = true; + private boolean closed; + private static final JsonNode SPAN_TYPES_SPEC = TestJsonSpec.getJson("span_types.json"); + static { transactionSchema = getSchema("/schema/transactions/transaction.json"); spanSchema = getSchema("/schema/transactions/span.json"); @@ -119,26 +127,43 @@ private static JsonSchema getSchema(String resource) { } /** - * @param enable {@literal true} to enable unknown outcome check, {@literal false} to allow for unknown outcome + * Sets all optional checks to their default value (enabled), should be used as a shortcut to reset mock reporter state + * after/before using it for a single test execution + */ + public void resetChecks() { + checkDestinationAddress = true; + checkUnknownOutcomes = true; + checkStrictSpanType = true; + } + + /** + * Disables unknown outcome check */ - public void checkUnknownOutcome(boolean enable) { - checkUnknownOutcomes = enable; + public void disableCheckUnknownOutcome() { + checkUnknownOutcomes = false; } - public void disableDestinationAddressCheck() { - disableDestinationAddressCheck = true; + /** + * Disables destination address check + */ + public void disableCheckDestinationAddress() { + checkDestinationAddress = false; } - public void enableDestinationAddressCheck() { - disableDestinationAddressCheck = false; + public boolean checkDestinationAddress() { + return checkDestinationAddress; } - public boolean isDisableDestinationAddressCheck() { - return disableDestinationAddressCheck; + /** + * Disables strict span type and sub-type check (against shared spec) + */ + public void disableCheckStrictSpanType() { + checkStrictSpanType = false; } @Override - public void start() {} + public void start() { + } @Override public synchronized void report(Transaction transaction) { @@ -164,19 +189,58 @@ public synchronized void report(Span span) { if (closed) { return; } - verifySpanSchema(asJson(dslJsonSerializer.toJsonString(span))); - verifyDestinationFields(span); + try { + verifySpanSchema(asJson(dslJsonSerializer.toJsonString(span))); + verifySpanType(span); + verifyDestinationFields(span); + + if (checkUnknownOutcomes) { + assertThat(span.getOutcome()) + .describedAs("span outcome should be either success or failure for type = %s", span.getType()) + .isNotEqualTo(Outcome.UNKNOWN); + } + } catch (Exception e) { + // in case a validation error occurs, ensure that it's properly logged for easier debugging + e.printStackTrace(System.err); + throw e; + } + + spans.add(span); + } + + + private void verifySpanType(Span span) { String type = span.getType(); - assertThat(type).isNotNull(); + assertThat(type) + .describedAs("span type is mandatory") + .isNotNull(); + + if (checkStrictSpanType) { + JsonNode typeJson = getMandatoryJson(SPAN_TYPES_SPEC, type, String.format("span type '%s' is not allowed by the spec", type)); + + boolean allowNullSubtype = getBooleanJson(typeJson, "allow_null_subtype"); + boolean allowUnlistedSubtype = getBooleanJson(typeJson, "allow_unlisted_subtype"); + + String subType = span.getSubtype(); + + JsonNode subTypesJson = typeJson.get("subtypes"); + boolean hasSubtypes = subTypesJson != null && !subTypesJson.isEmpty(); + + if (null == subType) { + if (hasSubtypes) { + assertThat(allowNullSubtype) + .describedAs("span type '%s' requires non-null subtype (allow_null_subtype=false)", type) + .isTrue(); + } + } else { + if (!allowUnlistedSubtype && hasSubtypes) { + getMandatoryJson(subTypesJson, subType, String.format("span subtype '%s' is not allowed by the sped for type '%s'", subType, type)); + } + } - if (checkUnknownOutcomes) { - assertThat(span.getOutcome()) - .describedAs("span outcome should be either success or failure for type = %s", type) - .isNotEqualTo(Outcome.UNKNOWN); } - spans.add(span); } private void verifyDestinationFields(Span span) { @@ -184,7 +248,7 @@ private void verifyDestinationFields(Span span) { return; } Destination destination = span.getContext().getDestination(); - if (!disableDestinationAddressCheck && !SPAN_TYPES_WITHOUT_ADDRESS.contains(span.getSubtype())) { + if (checkDestinationAddress && !SPAN_TYPES_WITHOUT_ADDRESS.contains(span.getSubtype())) { // see if this span's action is not supported for its subtype Collection unsupportedActions = SPAN_ACTIONS_WITHOUT_ADDRESS.getOrDefault(span.getSubtype(), Collections.emptySet()); if (!unsupportedActions.contains(span.getAction())) { @@ -421,7 +485,7 @@ public synchronized void assertRecycledAfterDecrementingReferences() { List spans = getSpans(); List spansToFlush = spans.stream() - .filter(s-> !hasEmptyTraceContext(s)) + .filter(s -> !hasEmptyTraceContext(s)) .collect(Collectors.toList()); transactionsToFlush.forEach(Transaction::decrementReferences); @@ -482,8 +546,29 @@ public void awaitUntilAsserted(long timeoutMs, ThrowingRunnable runnable) { } } - private static boolean hasEmptyTraceContext(AbstractSpan item) { return item.getTraceContext().getId().isEmpty(); } + + private static JsonNode getMandatoryJson(JsonNode json, String name, String desc) { + JsonNode jsonNode = json.get(name); + assertThat(jsonNode) + .describedAs(desc) + .isNotNull(); + assertThat(jsonNode.isObject()) + .isTrue(); + return jsonNode; + } + + private static boolean getBooleanJson(JsonNode json, String name) { + JsonNode jsonValue = json.get(name); + boolean value = false; + if (jsonValue != null) { + assertThat(jsonValue.isBoolean()) + .describedAs("property %s should be a boolean", name) + .isTrue(); + value = jsonValue.asBoolean(); + } + return value; + } } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/SpanTypeBreakdownTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/SpanTypeBreakdownTest.java index 688b31b231..53dd482fc7 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/SpanTypeBreakdownTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/SpanTypeBreakdownTest.java @@ -377,7 +377,12 @@ void testBreakdown_spanStartedAfterParentEnded() { } private void assertThatTransactionBreakdownCounterCreated(Map metricSets) { - assertThat(metricSets.get(Labels.Mutable.of().transactionName("test").transactionType("request")).getCounters().get("transaction.breakdown.count").get()).isEqualTo(1); + assertThat(metricSets.get(Labels.Mutable.of() + .transactionName("test") + .transactionType("request")) + .getCounters().get("transaction.breakdown.count") + .get()) + .isEqualTo(1); } private Transaction createTransaction() { @@ -388,7 +393,11 @@ private Transaction createTransaction() { @Nullable private Timer getTimer(Map metricSets, String timerName, @Nullable String spanType, @Nullable String spanSubType) { - final MetricSet metricSet = metricSets.get(Labels.Mutable.of().transactionName("test").transactionType("request").spanType(spanType).spanSubType(spanSubType)); + final MetricSet metricSet = metricSets.get(Labels.Mutable.of() + .transactionName("test") + .transactionType("request") + .spanType(spanType) + .spanSubType(spanSubType)); if (metricSet == null) { return null; } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/transaction/SpanTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/transaction/SpanTest.java index b40d93354b..1dbb408e8b 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/transaction/SpanTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/transaction/SpanTest.java @@ -26,7 +26,6 @@ import co.elastic.apm.agent.MockTracer; import org.junit.jupiter.api.Test; -import org.mockito.Mock; import static org.assertj.core.api.Assertions.assertThat; @@ -69,4 +68,14 @@ void testOutcomeExplicitlyToUnknown() { assertThat(span.getOutcome()).isEqualTo(Outcome.UNKNOWN); } + + @Test + void normalizeEmptyFields() { + Span span = new Span(MockTracer.create()) + .withName("span"); + + assertThat(span.withType("").getType()).isNull(); + assertThat(span.withSubtype("").getSubtype()).isNull(); + assertThat(span.withAction("").getAction()).isNull(); + } } diff --git a/apm-agent-core/src/test/resources/json-specs/span_types.json b/apm-agent-core/src/test/resources/json-specs/span_types.json new file mode 100644 index 0000000000..847872d8df --- /dev/null +++ b/apm-agent-core/src/test/resources/json-specs/span_types.json @@ -0,0 +1,138 @@ +{ + "__description": { + "": "root element for type identified by ''", + ".__description": "description for '' (optional)", + ".allow_null_subtype": "true to allow null subtype, false by default if omitted", + ".allow_unlisted_subtype": "true to allow unlisted subtypes, false by default if omitted", + ".subtypes": "root element for sub-types of type '', if omitted or empty subtype must be null, unless 'allow_unlisted_subtype' is set to true", + ".subtypes.": "sub-type element for ", + ".subtypes..__description": "description of subtype (optional)" + }, + "app": { + "allow_null_subtype": true, + "subtypes": { + "inferred": { + "__description": "Sampling profiler inferred spans" + } + } + }, + "custom": { + "__description": "API custom instrumentation", + "allow_null_subtype": true + }, + "db": { + "__description": "database span", + "subtypes": { + "cassandra": { + "__description": "Cassandra" + }, + "cosmosdb": { + "__description": "Azure CosmosDB" + }, + "db2": { + "__description": "IBM DB2" + }, + "derby": { + "__description": "Apache Derby" + }, + "dynamodb": { + "__description": "AWS DynamoDB" + }, + "elasticsearch": { + "__description": "Elasticsearch" + }, + "h2": { + "__description": "H2" + }, + "hsqldb": { + "__description": "HSQLDB" + }, + "ingres": { + "__description": "Ingres" + }, + "mariadb": { + "__description": "MariaDB" + }, + "mongodb": { + "__description": "MongoDB" + }, + "mysql": { + "__description": "MySQL" + }, + "oracle": { + "__description": "Oracle Database" + }, + "postgresql": { + "__description": "PostgreSQL" + }, + "redis": { + "__description": "Redis" + }, + "sqlserver": { + "__description": "Microsoft SQL Server" + }, + "unknown": { + "__description": "Unknown database" + } + } + }, + "external": { + "subtypes": { + "dubbo": { + "__description": "Apache Dubbo" + }, + "grpc": { + "__description": "gRPC" + }, + "http": { + "__description": "HTTP client" + } + } + }, + "messaging": { + "__description": "Messaging", + "subtypes": { + "azurequeue": { + "__description": "Azure Queue" + }, + "azureservicebus": { + "__description": "Azure Service Bus" + }, + "jms": { + "__description": "Java Messaging Service" + }, + "kafka": { + "__description": "Apache Kafka" + }, + "rabbitmq": { + "__description": "RabbitMQ" + }, + "sns": { + "__description": "AWS Simple Notification Service" + }, + "sqs": { + "__description": "AWS Simple Queue Service" + } + } + }, + "process": { + "__description": "External process" + }, + "storage": { + "subtypes": { + "azurefile": { + "__description": "Azure Files" + }, + "azuresblob": { + "__description": "Azure Blob Storage" + }, + "s3": { + "__description": "AWS S3" + } + } + }, + "template": { + "__description": "Template engines (no sub-type for now as really platform-specific)", + "allow_unlisted_subtype": true + } +} diff --git a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/CaptureSpanInstrumentation.java b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/CaptureSpanInstrumentation.java index cf32e698f9..cfc7b22b80 100644 --- a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/CaptureSpanInstrumentation.java +++ b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/CaptureSpanInstrumentation.java @@ -73,10 +73,10 @@ public static Object onMethodEnter( @Nullable @AnnotationValueOffsetMappingFactory.AnnotationValueExtractor(annotationClassName = "co.elastic.apm.api.CaptureSpan", method = "action") String action) { final AbstractSpan parent = tracer.getActive(); if (parent != null) { - Span span = parent.createSpan(); - span.setType(type, subtype, action); - span.withName(spanName.isEmpty() ? signature : spanName) + Span span = parent.createSpan() + .withName(spanName.isEmpty() ? signature : spanName) .activate(); + span.setType(type, subtype, action); return span; } else { logger.debug("Not creating span for {} because there is no currently active span.", signature); diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/SpanInstrumentationTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/SpanInstrumentationTest.java index 29438046b4..4c06c198dd 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/SpanInstrumentationTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/SpanInstrumentationTest.java @@ -67,7 +67,10 @@ void testSetName() { } @Test + @SuppressWarnings("deprecation") void testLegacyAPIs() { + reporter.disableCheckStrictSpanType(); + Span span = transaction.createSpan(); span.setType("foo.bar.baz"); endSpan(span); @@ -79,6 +82,8 @@ void testLegacyAPIs() { @Test void testTypes() { + reporter.disableCheckStrictSpanType(); + Span span = transaction.startSpan("foo", "bar", "baz"); endSpan(span); co.elastic.apm.agent.impl.transaction.Span internalSpan = reporter.getFirstSpan(); @@ -89,6 +94,8 @@ void testTypes() { @Test void testChaining() { + reporter.disableCheckStrictSpanType(); + int randomInt = random.nextInt(); boolean randomBoolean = random.nextBoolean(); String randomString = RandomStringUtils.randomAlphanumeric(3); diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentationTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentationTest.java index 6d196d2b0a..9c63b2047a 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentationTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentationTest.java @@ -165,7 +165,10 @@ void testChaining() { } @Test - public void startSpan() throws Exception { + public void startSpan() { + // custom spans not part of shared spec + reporter.disableCheckStrictSpanType(); + Span span = transaction.startSpan("foo", null, null); span.setName("bar"); Span child = span.startSpan("foo2", null, null); @@ -222,7 +225,7 @@ public void testGetErrorIdWithTransactionCaptureException() { @Test public void testGetErrorIdWithSpanCaptureException() { String errorId = null; - Span span = transaction.startSpan("foo", null, null); + Span span = transaction.startSpan("custom", null, null); span.setName("bar"); try { throw new RuntimeException("test exception"); @@ -237,7 +240,7 @@ public void testGetErrorIdWithSpanCaptureException() { @Test void setOutcome_unknown() { - reporter.checkUnknownOutcome(false); + reporter.disableCheckUnknownOutcome(); testSetOutcome(Outcome.UNKNOWN); } diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AnnotationApiTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AnnotationApiTest.java index 303e4db64e..7d21d5c634 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AnnotationApiTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AnnotationApiTest.java @@ -27,7 +27,6 @@ import co.elastic.apm.agent.AbstractInstrumentationTest; import co.elastic.apm.agent.impl.TracerInternalApiUtils; import co.elastic.apm.agent.impl.transaction.Span; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; @@ -83,6 +82,9 @@ void testType() { } private void testTransactionAndSpanTypes(boolean useLegacyTyping) { + + reporter.disableCheckStrictSpanType(); + reporter.reset(); AnnotationTestClass.transactionWithType(useLegacyTyping); assertThat(reporter.getTransactions()).hasSize(1); @@ -93,24 +95,29 @@ private void testTransactionAndSpanTypes(boolean useLegacyTyping) { assertThat(reporter.getSpans()).hasSize(1); Span internalSpan = reporter.getFirstSpan(); assertThat(internalSpan.getNameAsString()).isEqualTo("spanWithType"); - assertThat(internalSpan.getType()).isEqualTo("ext"); + assertThat(internalSpan.getType()).isEqualTo("external"); assertThat(internalSpan.getSubtype()).isEqualTo("http"); assertThat(internalSpan.getAction()).isEqualTo("okhttp"); } @Test void testMissingSubtype() { + reporter.disableCheckStrictSpanType(); + AnnotationTestClass.transactionForMissingSpanSubtype(); assertThat(reporter.getSpans()).hasSize(1); Span internalSpan = reporter.getFirstSpan(); assertThat(internalSpan.getNameAsString()).isEqualTo("spanWithMissingSubtype"); - assertThat(internalSpan.getType()).isEqualTo("ext.http"); - assertThat(internalSpan.getSubtype()).isEqualTo(""); + assertThat(internalSpan.getType()).isEqualTo("external.http"); + assertThat(internalSpan.getSubtype()).isNull(); assertThat(internalSpan.getAction()).isEqualTo("okhttp"); } @Test void testTracedTransactionAndSpan() { + // type = app, subtype = subtype is not part of shared spec + reporter.disableCheckStrictSpanType(); + TracedAnnotationTest.tracedTransaction(); assertThat(reporter.getTransactions()).hasSize(1); assertThat(reporter.getSpans()).hasSize(1); @@ -180,12 +187,12 @@ static void transactionWithType(boolean useLegacyTyping) { } } - @CaptureSpan(value = "spanWithType", type = "ext.http.okhttp") + @CaptureSpan(value = "spanWithType", type = "external.http.okhttp") private static void spanWithHierarchicalType() { } - @CaptureSpan(value = "spanWithType", type = "ext", subtype = "http", action = "okhttp") + @CaptureSpan(value = "spanWithType", type = "external", subtype = "http", action = "okhttp") private static void spanWithSplitTypes() { } @@ -195,7 +202,7 @@ static void transactionForMissingSpanSubtype() { spanWithMissingSubtype(); } - @CaptureSpan(value = "spanWithMissingSubtype", type = "ext.http", action = "okhttp") + @CaptureSpan(value = "spanWithMissingSubtype", type = "external.http", action = "okhttp") private static void spanWithMissingSubtype() { } diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/BlockingQueueContextPropagationTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/BlockingQueueContextPropagationTest.java index 5dfcc782d2..5cc38cb6be 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/BlockingQueueContextPropagationTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/BlockingQueueContextPropagationTest.java @@ -106,6 +106,9 @@ public void testAsyncTransactionDelegation() throws ExecutionException, Interrup @Test public void testAsyncSpanDelegation() throws ExecutionException, InterruptedException { + // span type/subtype 'async/blocking-queue' is not part of shared spec + reporter.disableCheckStrictSpanType(); + Transaction transaction = ElasticApm.startTransaction(); long startTime = TimeUnit.NANOSECONDS.toMicros(nanoTimeOffsetToEpoch + System.nanoTime()); transaction.setStartTimestamp(startTime); @@ -136,6 +139,7 @@ public void testAsyncSpanDelegation() throws ExecutionException, InterruptedExce assertThat(reportedSpan.getTraceContext().getTraceId()).isEqualTo(reportedTransaction.getTraceContext().getTraceId()); assertThat(reportedSpan).isNotNull(); assertThat(reportedSpan.getType()).isEqualTo("async"); + assertThat(reportedSpan.getSubtype()).isEqualTo("blocking-queue"); assertThat(reportedSpan.getTimestamp() - transactionTimestamp).isGreaterThanOrEqualTo(TimeUnit.MILLISECONDS.toMicros(100)); assertThat(reportedSpan.getDuration()).isBetween( TimeUnit.MILLISECONDS.toMicros(10), diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java index c50b786310..b409d7071c 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java @@ -29,7 +29,6 @@ import co.elastic.apm.agent.impl.TextHeaderMapAccessor; import co.elastic.apm.agent.impl.TracerInternalApiUtils; import co.elastic.apm.agent.impl.transaction.AbstractSpan; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.util.Collections; @@ -92,7 +91,7 @@ void testCaptureExceptionNoopSpan() { @Test void testTransactionWithError() { - reporter.checkUnknownOutcome(false); + reporter.disableCheckUnknownOutcome(); final Transaction transaction = ElasticApm.startTransaction(); transaction.setType("request"); @@ -158,7 +157,7 @@ void testGetId_distributedTracingEnabled() { assertThat(ElasticApm.currentTransaction().getTraceId()).isEqualTo(transaction.getTraceContext().getTraceId().toString()); assertThat(ElasticApm.currentSpan().getId()).isEqualTo(transaction.getTraceContext().getId().toString()); assertThat(ElasticApm.currentSpan().getTraceId()).isEqualTo(transaction.getTraceContext().getTraceId().toString()); - co.elastic.apm.agent.impl.transaction.Span span = transaction.createSpan().withType("db").withName("SELECT"); + co.elastic.apm.agent.impl.transaction.Span span = transaction.createSpan().withType("db").withSubtype("mysql").withName("SELECT"); try (Scope spanScope = span.activateInScope()) { assertThat(ElasticApm.currentSpan().getId()).isEqualTo(span.getTraceContext().getId().toString()); assertThat(ElasticApm.currentSpan().getTraceId()).isEqualTo(span.getTraceContext().getTraceId().toString()); diff --git a/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-7_x/src/test/java/co/elastic/apm/agent/es/restclient/v7_x/ElasticsearchRestClientInstrumentationIT.java b/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-7_x/src/test/java/co/elastic/apm/agent/es/restclient/v7_x/ElasticsearchRestClientInstrumentationIT.java index 8539049dc8..b4455f3943 100644 --- a/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-7_x/src/test/java/co/elastic/apm/agent/es/restclient/v7_x/ElasticsearchRestClientInstrumentationIT.java +++ b/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-7_x/src/test/java/co/elastic/apm/agent/es/restclient/v7_x/ElasticsearchRestClientInstrumentationIT.java @@ -25,6 +25,7 @@ package co.elastic.apm.agent.es.restclient.v7_x; import co.elastic.apm.agent.es.restclient.v6_4.AbstractEs6_4ClientInstrumentationTest; +import co.elastic.apm.agent.impl.transaction.Outcome; import co.elastic.apm.agent.impl.transaction.Span; import org.apache.http.HttpHost; import org.apache.http.auth.AuthScope; @@ -65,8 +66,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; import java.util.List; @@ -76,7 +75,6 @@ @RunWith(Parameterized.class) public class ElasticsearchRestClientInstrumentationIT extends AbstractEs6_4ClientInstrumentationTest { - private static final Logger logger = LoggerFactory.getLogger(ElasticsearchRestClientInstrumentationIT.class); private static final String ELASTICSEARCH_CONTAINER_VERSION = "docker.elastic.co/elasticsearch/elasticsearch:7.11.0"; @@ -112,9 +110,9 @@ public static void stopElasticsearchContainerAndClient() throws IOException { @Test public void testCancelScenario() throws InterruptedException, ExecutionException, IOException { // When spans are cancelled, we can't know the actual address, because there is no response, and we set the outcome as UNKNOWN - reporter.disableDestinationAddressCheck(); - reporter.checkUnknownOutcome(false); - super.disableHttpUrlCheck(); + reporter.disableCheckDestinationAddress(); + reporter.disableCheckUnknownOutcome(); + disableHttpUrlCheck(); createDocument(); reporter.reset(); @@ -140,11 +138,11 @@ public void onFailure(Exception e) { Span searchSpan = spans.get(0); validateSpanContent(searchSpan, String.format("Elasticsearch: POST /%s/_search", INDEX), -1, "POST"); - deleteDocument(); + assertThat(searchSpan.getOutcome()) + .describedAs("span outcome should be unknown when cancelled") + .isEqualTo(Outcome.UNKNOWN); - reporter.enableDestinationAddressCheck(); - reporter.checkUnknownOutcome(true); - super.enableHttpUrlCheck(); + deleteDocument(); } @Override diff --git a/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-common/src/test/java/co/elastic/apm/agent/es/restclient/AbstractEsClientInstrumentationTest.java b/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-common/src/test/java/co/elastic/apm/agent/es/restclient/AbstractEsClientInstrumentationTest.java index ff1735680c..bb9375ea47 100644 --- a/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-common/src/test/java/co/elastic/apm/agent/es/restclient/AbstractEsClientInstrumentationTest.java +++ b/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-common/src/test/java/co/elastic/apm/agent/es/restclient/AbstractEsClientInstrumentationTest.java @@ -63,14 +63,13 @@ public abstract class AbstractEsClientInstrumentationTest extends AbstractInstru protected boolean async; - private boolean disableHttpUrlCheck = false; + private boolean checkHttpUrl = true; + /** + * Disables HTTP URL check for the current test method + */ public void disableHttpUrlCheck() { - disableHttpUrlCheck = true; - } - - public void enableHttpUrlCheck() { - disableHttpUrlCheck = false; + checkHttpUrl = false; } @Parameterized.Parameters(name = "Async={0}") @@ -93,6 +92,12 @@ public static void stopContainer() { @Before public void startTransaction() { + // While JUnit does not recycle test class instances between method invocations by default + // this test should not be required, but it allows to ensure proper correctness even if that changes + assertThat(checkHttpUrl) + .describedAs("checking HTTP URLs should be enabled by default") + .isTrue(); + startTestRootTransaction("ES Transaction"); } @@ -105,7 +110,6 @@ public void endTransaction() { } public void assertThatErrorsExistWhenDeleteNonExistingIndex() { - List errorCaptures = reporter.getErrors(); assertThat(errorCaptures).hasSize(1); ErrorCapture errorCapture = errorCaptures.get(0); @@ -130,7 +134,6 @@ protected void validateDbContextContent(Span span, String statement) { assertThat(db.getStatementBuffer().toString()).isEqualTo(statement); } - protected void validateSpanContent(Span span, String expectedName, int statusCode, String method) { validateSpanContentWithoutContext(span, expectedName, statusCode, method); validateHttpContextContent(span.getContext().getHttp(), statusCode, method); @@ -139,10 +142,12 @@ protected void validateSpanContent(Span span, String expectedName, int statusCod private void validateDestinationContextContent(Destination destination) { assertThat(destination).isNotNull(); - if (!reporter.isDisableDestinationAddressCheck()) { + + if (reporter.checkDestinationAddress()) { assertThat(destination.getAddress().toString()).isEqualTo(container.getContainerIpAddress()); assertThat(destination.getPort()).isEqualTo(container.getMappedPort(9200)); } + assertThat(destination.getService().getName().toString()).isEqualTo(ELASTICSEARCH); assertThat(destination.getService().getResource().toString()).isEqualTo(ELASTICSEARCH); assertThat(destination.getService().getType()).isEqualTo(SPAN_TYPE); @@ -152,27 +157,24 @@ private void validateHttpContextContent(Http http, int statusCode, String method assertThat(http).isNotNull(); assertThat(http.getMethod()).isEqualTo(method); assertThat(http.getStatusCode()).isEqualTo(statusCode); - if (!disableHttpUrlCheck) { + if (checkHttpUrl) { assertThat(http.getUrl()).isEqualTo("http://" + container.getHttpHostAddress()); } } protected void validateSpanContentAfterIndexCreateRequest() { - List spans = reporter.getSpans(); assertThat(spans).hasSize(1); validateSpanContent(spans.get(0), String.format("Elasticsearch: PUT /%s", SECOND_INDEX), 200, "PUT"); } protected void validateSpanContentAfterIndexDeleteRequest() { - List spans = reporter.getSpans(); assertThat(spans).hasSize(1); validateSpanContent(spans.get(0), String.format("Elasticsearch: DELETE /%s", SECOND_INDEX), 200, "DELETE"); } protected void validateSpanContentAfterBulkRequest() { - List spans = reporter.getSpans(); assertThat(spans).hasSize(1); assertThat(spans.get(0).getNameAsString()).isEqualTo("Elasticsearch: POST /_bulk"); diff --git a/apm-agent-plugins/apm-hibernate-search-plugin/apm-hibernate-search-plugin-5_x/src/test/java/co/elastic/apm/agent/hibernatesearch/HibernateSearch5InstrumentationTest.java b/apm-agent-plugins/apm-hibernate-search-plugin/apm-hibernate-search-plugin-5_x/src/test/java/co/elastic/apm/agent/hibernatesearch/HibernateSearch5InstrumentationTest.java index 986f44797b..6fbca3325e 100644 --- a/apm-agent-plugins/apm-hibernate-search-plugin/apm-hibernate-search-plugin-5_x/src/test/java/co/elastic/apm/agent/hibernatesearch/HibernateSearch5InstrumentationTest.java +++ b/apm-agent-plugins/apm-hibernate-search-plugin/apm-hibernate-search-plugin-5_x/src/test/java/co/elastic/apm/agent/hibernatesearch/HibernateSearch5InstrumentationTest.java @@ -74,6 +74,8 @@ static void setUp() throws IOException { @BeforeEach void initSingleTest() { + // hibernate is not in the shared span type/subtype specification + reporter.disableCheckStrictSpanType(); tracer.startRootTransaction(null).activate(); } diff --git a/apm-agent-plugins/apm-hibernate-search-plugin/apm-hibernate-search-plugin-6_x/src/test/java/co/elastic/apm/agent/hibernatesearch/HibernateSearch6InstrumentationTest.java b/apm-agent-plugins/apm-hibernate-search-plugin/apm-hibernate-search-plugin-6_x/src/test/java/co/elastic/apm/agent/hibernatesearch/HibernateSearch6InstrumentationTest.java index 16bf99ae6b..889c0ccc5b 100644 --- a/apm-agent-plugins/apm-hibernate-search-plugin/apm-hibernate-search-plugin-6_x/src/test/java/co/elastic/apm/agent/hibernatesearch/HibernateSearch6InstrumentationTest.java +++ b/apm-agent-plugins/apm-hibernate-search-plugin/apm-hibernate-search-plugin-6_x/src/test/java/co/elastic/apm/agent/hibernatesearch/HibernateSearch6InstrumentationTest.java @@ -67,6 +67,9 @@ static void setUp() throws IOException { @BeforeEach void initSingleTest() { + // hibernate search is not part of shared spec + reporter.disableCheckStrictSpanType(); + tracer.startRootTransaction(null).activate(); } diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java index b0dde1cada..645cb68c71 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java @@ -117,19 +117,21 @@ public Span createJdbcSpan(@Nullable String sql, Object statement, @Nullable Abs Connection connection = safeGetConnection((Statement) statement); ConnectionMetaData connectionMetaData = connection == null ? null : getConnectionMetaData(connection); + + String vendor = "unknown"; if (connectionMetaData != null) { - span.withSubtype(connectionMetaData.getDbVendor()) - .withAction(DB_SPAN_ACTION); + vendor = connectionMetaData.getDbVendor();; span.getContext().getDb() .withUser(connectionMetaData.getUser()); Destination destination = span.getContext().getDestination() .withAddress(connectionMetaData.getHost()) .withPort(connectionMetaData.getPort()); destination.getService() - .withName(connectionMetaData.getDbVendor()) - .withResource(connectionMetaData.getDbVendor()) + .withName(vendor) + .withResource(vendor) .withType(DB_SPAN_TYPE); } + span.withSubtype(vendor).withAction(DB_SPAN_ACTION); return span; } diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/AbstractJdbcInstrumentationTest.java b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/AbstractJdbcInstrumentationTest.java index 6bf902964d..d6d1fa91b9 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/AbstractJdbcInstrumentationTest.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/AbstractJdbcInstrumentationTest.java @@ -223,6 +223,7 @@ private void checkWithoutConnectionMetadata(TestStatement statement, ThrownCount final String sql = "UPDATE ELASTIC_APM SET BAR='AFTER1' WHERE FOO=11"; boolean isResultSet = statement.execute(sql); + assertThat(check.getThrownCount()).isEqualTo(1); assertThat(isResultSet).isFalse(); @@ -230,6 +231,7 @@ private void checkWithoutConnectionMetadata(TestStatement statement, ThrownCount // try to execute statement again, should not throw again statement.execute(sql); + assertThat(check.getThrownCount()) .describedAs("unsupported exception should only be thrown once") .isEqualTo(1); @@ -442,8 +444,8 @@ private void assertSpanRecordedWithoutConnection(String rawSql, boolean prepared signatureParser.querySignature(rawSql, processedSql, preparedStatement); assertThat(jdbcSpan.getNameAsString()).isEqualTo(processedSql.toString()); assertThat(jdbcSpan.getType()).isEqualTo(DB_SPAN_TYPE); - assertThat(jdbcSpan.getSubtype()).isNull(); - assertThat(jdbcSpan.getAction()).isNull(); + assertThat(jdbcSpan.getSubtype()).isEqualTo("unknown"); + assertThat(jdbcSpan.getAction()).isEqualTo(DB_SPAN_ACTION); Db db = jdbcSpan.getContext().getDb(); assertThat(db.getStatement()).isEqualTo(rawSql); diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/helper/ConnectionMetaDataTest.java b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/helper/ConnectionMetaDataTest.java index 0957fc7fcc..5fcebe7920 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/helper/ConnectionMetaDataTest.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/helper/ConnectionMetaDataTest.java @@ -24,11 +24,15 @@ */ package co.elastic.apm.agent.jdbc.helper; +import co.elastic.apm.agent.MockReporter; +import co.elastic.apm.agent.MockTracer; +import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.impl.transaction.Transaction; import org.junit.jupiter.api.Test; import javax.annotation.Nullable; -import static org.junit.jupiter.api.Assertions.*; +import static org.assertj.core.api.Assertions.assertThat; class ConnectionMetaDataTest { @@ -317,10 +321,38 @@ void testInvalid() { testUrl("postgresql://myhost:666/database", "unknown", null, -1); } + private static final MockTracer.MockInstrumentationSetup mockSetup = MockTracer.createMockInstrumentationSetup(); + private static MockReporter reporter = mockSetup.getReporter(); + private static ElasticApmTracer tracer = mockSetup.getTracer(); + private void testUrl(String url, String expectedVendor, @Nullable String expectedHost, int expectedPort) { ConnectionMetaData metadata = ConnectionMetaData.create(url, "TEST_USER"); - assertEquals(metadata.getDbVendor(), expectedVendor); - assertEquals(metadata.getHost(), expectedHost); - assertEquals(metadata.getPort(), expectedPort); + assertThat(metadata.getDbVendor()).isEqualTo(expectedVendor); + assertThat(metadata.getHost()).isEqualTo(expectedHost); + assertThat(metadata.getPort()).isEqualTo(expectedPort); + + if ("arbitrary".equals(expectedVendor)) { + // known case where the jdbc url is valid, but has an unknwon value in shared spec + return; + } + + // try to create a DB span in order to trigger shared span type validation logic + // given not all JDBC urls listed here will be explicitly tested with a real database, that allows + // to make sure that at least the known ones always fit + try { + + Transaction transaction = tracer.startRootTransaction(this.getClass().getClassLoader()); + assertThat(transaction).isNotNull(); + transaction.createSpan() + .withType("db") + .withSubtype(metadata.getDbVendor()).end(); + transaction.end(); + + assertThat(reporter.getNumReportedSpans()).isEqualTo(1); + assertThat(reporter.getNumReportedTransactions()).isEqualTo(1); + } finally { + reporter.reset(); + } + } } diff --git a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-base-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyClientIT.java b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-base-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyClientIT.java index 50e1874090..a45b33da08 100644 --- a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-base-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyClientIT.java +++ b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-base-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyClientIT.java @@ -241,7 +241,7 @@ public void testDestinationAddressCollectionDisabled() { when(messagingConfiguration.shouldCollectQueueAddress()).thenReturn(false); testScenario = TestScenario.TOPIC_ADDRESS_COLLECTION_DISABLED; consumerThread.setIterationMode(RecordIterationMode.ITERABLE_FOR); - reporter.disableDestinationAddressCheck(); + reporter.disableCheckDestinationAddress(); sendTwoRecordsAndConsumeReplies(); verifyTracing(); } diff --git a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaIT.java b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaIT.java index 8ab377b654..b8a9642885 100644 --- a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaIT.java +++ b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaIT.java @@ -57,7 +57,6 @@ import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; -import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.KafkaContainer; import org.testcontainers.shaded.com.google.common.collect.ImmutableMap; @@ -264,7 +263,7 @@ public void testDestinationAddressCollectionDisabled() { when(messagingConfiguration.shouldCollectQueueAddress()).thenReturn(false); testScenario = TestScenario.TOPIC_ADDRESS_COLLECTION_DISABLED; consumerThread.setIterationMode(RecordIterationMode.ITERABLE_FOR); - reporter.disableDestinationAddressCheck(); + reporter.disableCheckDestinationAddress(); sendTwoRecordsAndConsumeReplies(); verifyTracing(); } diff --git a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyBrokerIT.java b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyBrokerIT.java index cdc2dbff92..6e6df6a584 100644 --- a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyBrokerIT.java +++ b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyBrokerIT.java @@ -107,7 +107,7 @@ public KafkaLegacyBrokerIT() { @BeforeClass public static void setup() { - reporter.disableDestinationAddressCheck(); + reporter.disableCheckDestinationAddress(); // confluent versions 3.2.x correspond Kafka versions 0.10.2.2 - // https://docs.confluent.io/current/installation/versions-interoperability.html#cp-and-apache-ak-compatibility @@ -225,7 +225,7 @@ public void testDestinationAddressCollectionDisabled() { when(messagingConfiguration.shouldCollectQueueAddress()).thenReturn(false); testScenario = TestScenario.TOPIC_ADDRESS_COLLECTION_DISABLED; consumerThread.setIterationMode(RecordIterationMode.ITERABLE_FOR); - reporter.disableDestinationAddressCheck(); + reporter.disableCheckDestinationAddress(); sendTwoRecordsAndConsumeReplies(); verifyTracing(); } diff --git a/apm-agent-plugins/apm-opentracing-plugin/src/main/java/co/elastic/apm/agent/opentracingimpl/ApmSpanInstrumentation.java b/apm-agent-plugins/apm-opentracing-plugin/src/main/java/co/elastic/apm/agent/opentracingimpl/ApmSpanInstrumentation.java index 60c6a1c267..279e2e27af 100644 --- a/apm-agent-plugins/apm-opentracing-plugin/src/main/java/co/elastic/apm/agent/opentracingimpl/ApmSpanInstrumentation.java +++ b/apm-agent-plugins/apm-opentracing-plugin/src/main/java/co/elastic/apm/agent/opentracingimpl/ApmSpanInstrumentation.java @@ -71,7 +71,7 @@ public FinishInstrumentation() { @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) public static void finishInternal(@Advice.FieldValue(value = "dispatcher", typing = Assigner.Typing.DYNAMIC) @Nullable Object context, - @Advice.Argument(0) long finishMicros) { + @Advice.Argument(0) long finishMicros) { if (context instanceof AbstractSpan) { doFinishInternal((AbstractSpan) context, finishMicros); } @@ -84,17 +84,9 @@ public static void doFinishInternal(AbstractSpan abstractSpan, long finishMic if (transaction.getType() == null) { if (transaction.getContext().getRequest().hasContent()) { transaction.withType(Transaction.TYPE_REQUEST); - } else { - transaction.withType("unknown"); } } - } else { - Span span = (Span) abstractSpan; - if (span.getType() == null) { - span.withType("unknown"); - } } - if (finishMicros >= 0) { abstractSpan.end(finishMicros); } else { @@ -256,11 +248,7 @@ private static boolean handleSpecialSpanTag(Span span, String key, Object value) return true; } else if ("db.type".equals(key)) { span.getContext().getDb().withType(value.toString()); - if (isCache(value)) { - span.withType("cache").withSubtype(value.toString()); - } else { - span.withType("db").withSubtype(value.toString()); - } + span.withType("db").withSubtype(value.toString()); return true; } else if ("db.instance".equals(key)) { span.getContext().getDb().withInstance(value.toString()); @@ -271,13 +259,14 @@ private static boolean handleSpecialSpanTag(Span span, String key, Object value) return true; } else if ("span.kind".equals(key)) { if (span.getType() == null && ("producer".equals(value) || "client".equals(value))) { - span.withType("ext"); + span.withType("external"); } return true; } else if ("http.status_code".equals(key) && value instanceof Number) { - int status =((Number) value).intValue(); + int status = ((Number) value).intValue(); span.getContext().getHttp().withStatusCode(status); - span.withOutcome(ResultUtil.getOutcomeByHttpClientStatus(status)); + span.withSubtype("http") + .withOutcome(ResultUtil.getOutcomeByHttpClientStatus(status)); return true; } else if ("http.url".equals(key) && value instanceof String) { span.getContext().getHttp().withUrl((String) value); @@ -289,9 +278,6 @@ private static boolean handleSpecialSpanTag(Span span, String key, Object value) return false; } - private static boolean isCache(Object dbType) { - return "redis".equals(dbType); - } } public static class GetTraceContextInstrumentation extends ApmSpanInstrumentation { diff --git a/apm-agent-plugins/apm-process-plugin/src/main/java/co/elastic/apm/agent/process/ProcessHelper.java b/apm-agent-plugins/apm-process-plugin/src/main/java/co/elastic/apm/agent/process/ProcessHelper.java index 7aa3e44cf7..d8606bc364 100644 --- a/apm-agent-plugins/apm-process-plugin/src/main/java/co/elastic/apm/agent/process/ProcessHelper.java +++ b/apm-agent-plugins/apm-process-plugin/src/main/java/co/elastic/apm/agent/process/ProcessHelper.java @@ -72,8 +72,6 @@ void doStartProcess(@Nonnull AbstractSpan parentContext, @Nonnull Process pro Span span = parentContext.createSpan() .withType("process") - .withSubtype(binaryName) - .withAction("execute") .withName(binaryName); // We don't require span to be activated as the background process is not really linked to current thread diff --git a/apm-agent-plugins/apm-process-plugin/src/test/java/co/elastic/apm/agent/process/ProcessHelperTest.java b/apm-agent-plugins/apm-process-plugin/src/test/java/co/elastic/apm/agent/process/ProcessHelperTest.java index 0d8bf9bbc3..b1b75b4613 100644 --- a/apm-agent-plugins/apm-process-plugin/src/test/java/co/elastic/apm/agent/process/ProcessHelperTest.java +++ b/apm-agent-plugins/apm-process-plugin/src/test/java/co/elastic/apm/agent/process/ProcessHelperTest.java @@ -80,8 +80,8 @@ void checkSpanNaming() { assertThat(span.getNameAsString()).isEqualTo(binaryName); assertThat(span.getType()).isEqualTo("process"); - assertThat(span.getSubtype()).isEqualTo(binaryName); - assertThat(span.getAction()).isEqualTo("execute"); + assertThat(span.getSubtype()).isNull(); + assertThat(span.getAction()).isNull(); assertThat(span.getOutcome()).isEqualTo(Outcome.SUCCESS); } @@ -178,7 +178,7 @@ void destroyWithoutProcessTerminatedShouldEndSpan() { verifyNoMoreInteractions(process); // we should not even use any method of process // we have to opt-in to allow unknown outcome - reporter.checkUnknownOutcome(false); + reporter.disableCheckUnknownOutcome(); helper.doStartProcess(transaction, process, "hello"); diff --git a/apm-agent-plugins/apm-profiling-plugin/src/test/java/co/elastic/apm/agent/profiler/SamplingProfilerTest.java b/apm-agent-plugins/apm-profiling-plugin/src/test/java/co/elastic/apm/agent/profiler/SamplingProfilerTest.java index a9851894d3..f47c9a275d 100644 --- a/apm-agent-plugins/apm-profiling-plugin/src/test/java/co/elastic/apm/agent/profiler/SamplingProfilerTest.java +++ b/apm-agent-plugins/apm-profiling-plugin/src/test/java/co/elastic/apm/agent/profiler/SamplingProfilerTest.java @@ -216,7 +216,7 @@ void testPostProcessingDisabled() throws Exception { } private void aInferred(Transaction transaction) throws Exception { - Span span = transaction.createSpan().withName("bExplicit").withType("test"); + Span span = transaction.createSpan().withName("bExplicit").withType("custom"); try (Scope spanScope = span.activateInScope()) { cInferred(); } finally { diff --git a/apm-agent-plugins/apm-rabbitmq/apm-rabbitmq-spring/src/test/java/co/elastic/apm/agent/rabbitmq/AbstractRabbitMqTest.java b/apm-agent-plugins/apm-rabbitmq/apm-rabbitmq-spring/src/test/java/co/elastic/apm/agent/rabbitmq/AbstractRabbitMqTest.java index 3534d09cf7..0af12bc4e2 100644 --- a/apm-agent-plugins/apm-rabbitmq/apm-rabbitmq-spring/src/test/java/co/elastic/apm/agent/rabbitmq/AbstractRabbitMqTest.java +++ b/apm-agent-plugins/apm-rabbitmq/apm-rabbitmq-spring/src/test/java/co/elastic/apm/agent/rabbitmq/AbstractRabbitMqTest.java @@ -48,6 +48,7 @@ public void verifyThatTransactionWithSpanCreated() { checkTransaction(receiveTransaction, TOPIC_EXCHANGE_NAME, "Spring AMQP"); Span testSpan = reporter.getFirstSpan(1000); assertThat(testSpan.getNameAsString()).isEqualTo("testSpan"); + assertThat(testSpan.getType()).isEqualTo("custom"); checkParentChild(receiveTransaction, testSpan); } @@ -69,7 +70,7 @@ public void verifyThatTransactionWithSpanCreated_DistributedTracing() { checkSendSpan(sendSpan, TOPIC_EXCHANGE_NAME, LOCALHOST_ADDRESS, container.getAmqpPort()); checkParentChild(sendSpan, receiveTransaction); - Span testSpan = spans.stream().filter(span -> span.getType().equals("http")).findFirst().get(); + Span testSpan = spans.stream().filter(span -> span.getType().equals("custom")).findFirst().get(); assertThat(testSpan.getNameAsString()).isEqualTo("testSpan"); checkParentChild(receiveTransaction, testSpan); } diff --git a/apm-agent-plugins/apm-rabbitmq/apm-rabbitmq-spring/src/test/java/co/elastic/apm/agent/rabbitmq/config/BaseConfiguration.java b/apm-agent-plugins/apm-rabbitmq/apm-rabbitmq-spring/src/test/java/co/elastic/apm/agent/rabbitmq/config/BaseConfiguration.java index 7fbc84da06..29d1f7c628 100644 --- a/apm-agent-plugins/apm-rabbitmq/apm-rabbitmq-spring/src/test/java/co/elastic/apm/agent/rabbitmq/config/BaseConfiguration.java +++ b/apm-agent-plugins/apm-rabbitmq/apm-rabbitmq-spring/src/test/java/co/elastic/apm/agent/rabbitmq/config/BaseConfiguration.java @@ -69,7 +69,7 @@ public RabbitTemplate rabbitTemplate(final ConnectionFactory connectionFactory) return rabbitTemplate; } - @CaptureSpan(value = "testSpan", type = "http", subtype = "get", action = "test") + @CaptureSpan(value = "testSpan", type = "custom", subtype = "anything", action = "test") public void testSpan() { } } diff --git a/apm-agent-plugins/apm-redis-plugin/apm-lettuce-3-tests/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce3InstrumentationTest.java b/apm-agent-plugins/apm-redis-plugin/apm-lettuce-3-tests/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce3InstrumentationTest.java index 8d174ef289..c9efb9efd6 100644 --- a/apm-agent-plugins/apm-redis-plugin/apm-lettuce-3-tests/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce3InstrumentationTest.java +++ b/apm-agent-plugins/apm-redis-plugin/apm-lettuce-3-tests/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce3InstrumentationTest.java @@ -44,7 +44,7 @@ public class Lettuce3InstrumentationTest extends AbstractRedisInstrumentationTes public void setUpLettuce() { client = new RedisClient("localhost", redisPort); connection = client.connect(); - reporter.disableDestinationAddressCheck(); + reporter.disableCheckDestinationAddress(); } @Test diff --git a/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce4InstrumentationTest.java b/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce4InstrumentationTest.java index d41ea88336..44e401c0f3 100644 --- a/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce4InstrumentationTest.java +++ b/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce4InstrumentationTest.java @@ -44,7 +44,7 @@ public class Lettuce4InstrumentationTest extends AbstractRedisInstrumentationTes public void setUpLettuce() { client = RedisClient.create("redis://localhost:" + redisPort); connection = client.connect(); - reporter.disableDestinationAddressCheck(); + reporter.disableCheckDestinationAddress(); } @Test diff --git a/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce5InstrumentationTest.java b/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce5InstrumentationTest.java index 8c4a14777a..491dbf4c5e 100644 --- a/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce5InstrumentationTest.java +++ b/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce5InstrumentationTest.java @@ -50,7 +50,7 @@ public class Lettuce5InstrumentationTest extends AbstractRedisInstrumentationTes public void setUpLettuce() { RedisClient client = RedisClient.create(RedisURI.create("localhost", redisPort)); connection = client.connect(); - reporter.disableDestinationAddressCheck(); + reporter.disableCheckDestinationAddress(); } @Test diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletInstrumentationTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletInstrumentationTest.java index 84540989c9..e18b2a1e54 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletInstrumentationTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletInstrumentationTest.java @@ -31,6 +31,7 @@ import okhttp3.Response; import org.eclipse.jetty.servlet.ErrorPageErrorHandler; import org.eclipse.jetty.servlet.ServletContextHandler; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import javax.servlet.DispatcherType; @@ -61,6 +62,12 @@ class ServletInstrumentationTest extends AbstractServletTest { + @BeforeEach + void beforeEach() { + // servlet type & subtype are nor part of shared spec + reporter.disableCheckStrictSpanType(); + } + @Override protected void setUpHandler(ServletContextHandler handler) { handler.setDisplayName(getClass().getSimpleName()); diff --git a/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeTest.java b/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeTest.java index 68ab7552b1..724d8499d2 100644 --- a/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeTest.java +++ b/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeTest.java @@ -300,12 +300,15 @@ public void testAgentPaused() { @Test void testResolveClientType() { + // does not fit the shared type/subtype spec + reporter.disableCheckStrictSpanType(); + assertSoftly(softly -> { - softly.assertThat(createSpanFromOtTags(Map.of("span.kind", "client")).getType()).isEqualTo("ext"); - softly.assertThat(createSpanFromOtTags(Map.of("span.kind", "producer")).getType()).isEqualTo("ext"); + softly.assertThat(createSpanFromOtTags(Map.of("span.kind", "client")).getType()).isEqualTo("external"); + softly.assertThat(createSpanFromOtTags(Map.of("span.kind", "producer")).getType()).isEqualTo("external"); softly.assertThat(createSpanFromOtTags(Map.of("span.kind", "client", "db.type", "mysql")).getType()).isEqualTo("db"); - softly.assertThat(createSpanFromOtTags(Map.of("span.kind", "client", "db.type", "foo")).getType()).isEqualTo("db"); - softly.assertThat(createSpanFromOtTags(Map.of("span.kind", "client", "db.type", "redis")).getType()).isEqualTo("cache"); + softly.assertThat(createSpanFromOtTags(Map.of("span.kind", "client", "db.type", "sqlserver")).getType()).isEqualTo("db"); + softly.assertThat(createSpanFromOtTags(Map.of("span.kind", "client", "db.type", "redis")).getType()).isEqualTo("db"); }); } @@ -330,7 +333,7 @@ void testTypeTags() { @Test void testResolveServerType() { assertSoftly(softly -> { - softly.assertThat(createTransactionFromOtTags(Map.of("span.kind", "server")).getType()).isEqualTo("unknown"); + softly.assertThat(createTransactionFromOtTags(Map.of("span.kind", "server")).getType()).isEqualTo("custom"); softly.assertThat(createTransactionFromOtTags(Map.of("span.kind", "server", "http.url", "http://localhost:8080", "http.method", "GET")).getType()).isEqualTo("request"); diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java index c3e87c4c1f..50c1ce0edf 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java @@ -124,7 +124,7 @@ private void testExecuteCommand(AbstractServletContainerIntegrationTest test, St for (JsonNode span : spans) { assertThat(span.get("parent_id").textValue()).isEqualTo(transactionId); assertThat(span.get("name").asText()).isEqualTo("java"); - assertThat(span.get("type").asText()).isEqualTo("process.java.execute"); + assertThat(span.get("type").asText()).isEqualTo("process"); } } diff --git a/integration-tests/external-plugin-test/external-plugin/src/test/java/co/elastic/apm/agent/plugin/PluginInstrumentationTest.java b/integration-tests/external-plugin-test/external-plugin/src/test/java/co/elastic/apm/agent/plugin/PluginInstrumentationTest.java index 49553253fc..3b1c5c2065 100644 --- a/integration-tests/external-plugin-test/external-plugin/src/test/java/co/elastic/apm/agent/plugin/PluginInstrumentationTest.java +++ b/integration-tests/external-plugin-test/external-plugin/src/test/java/co/elastic/apm/agent/plugin/PluginInstrumentationTest.java @@ -25,6 +25,7 @@ package co.elastic.apm.agent.plugin; import co.elastic.apm.agent.AbstractInstrumentationTest; +import co.elastic.apm.agent.impl.transaction.Span; import co.elastic.apm.agent.impl.transaction.Transaction; import co.elastic.apm.plugin.test.TestClass; import org.junit.jupiter.api.Test; @@ -57,6 +58,9 @@ void testTransactionCreation() { @Test void testSpanCreation() { + // using custom span type/sub-type not part of shared spec + reporter.disableCheckStrictSpanType(); + Transaction transaction = tracer.startRootTransaction(null); Objects.requireNonNull(transaction).activate() .withName("Plugin test transaction") @@ -65,7 +69,11 @@ void testSpanCreation() { transaction.deactivate().end(); assertThat(reporter.getTransactions()).hasSize(1); assertThat(reporter.getSpans()).hasSize(1); - assertThat(reporter.getFirstSpan().getNameAsString()).isEqualTo("traceMe"); + Span span = reporter.getFirstSpan(); + assertThat(span.getNameAsString()).isEqualTo("traceMe"); + assertThat(span.getType()).isEqualTo("plugin"); + assertThat(span.getSubtype()).isEqualTo("external"); + assertThat(span.getAction()).isEqualTo("trace"); assertThat(reporter.getErrors()).isEmpty(); }