From c64c7e96cd20abe4e4355d8392e194a99a0f5a87 Mon Sep 17 00:00:00 2001 From: Tobias Stadler Date: Sat, 10 Jul 2021 10:53:08 +0200 Subject: [PATCH 1/7] Added support for setting the framework using the public api (#1908) --- .../java/co/elastic/apm/api/NoopTransaction.java | 9 +++++++++ .../main/java/co/elastic/apm/api/Transaction.java | 13 +++++++++++++ .../java/co/elastic/apm/api/TransactionImpl.java | 7 +++++++ .../pluginapi/TransactionInstrumentation.java | 14 ++++++++++++++ ...elastic.apm.agent.sdk.ElasticApmInstrumentation | 1 + .../pluginapi/TransactionInstrumentationTest.java | 8 +++++++- 6 files changed, 51 insertions(+), 1 deletion(-) diff --git a/apm-agent-api/src/main/java/co/elastic/apm/api/NoopTransaction.java b/apm-agent-api/src/main/java/co/elastic/apm/api/NoopTransaction.java index d7745ec83c..fe66caf0da 100644 --- a/apm-agent-api/src/main/java/co/elastic/apm/api/NoopTransaction.java +++ b/apm-agent-api/src/main/java/co/elastic/apm/api/NoopTransaction.java @@ -32,6 +32,8 @@ public Transaction setName(String name) { return this; } + + @Nonnull @Override public Transaction setType(String type) { @@ -39,6 +41,13 @@ public Transaction setType(String type) { return this; } + @Nonnull + @Override + public Transaction setFrameworkName(String frameworkName) { + // noop + return this; + } + @Nonnull @Deprecated @Override diff --git a/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java b/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java index eec5379770..1931119da9 100644 --- a/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java +++ b/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java @@ -57,6 +57,19 @@ public interface Transaction extends Span { @Nonnull Transaction setType(String type); + /** + * Override the name of the framework for the current transaction. + *

+ * For supported frameworks, + * the framework name is determined automatically, + * and can be overridden using this function. + *

+ * + * @param frameworkName The name of the framework + */ + @Nonnull + Transaction setFrameworkName(String frameworkName); + /** * {@inheritDoc} * diff --git a/apm-agent-api/src/main/java/co/elastic/apm/api/TransactionImpl.java b/apm-agent-api/src/main/java/co/elastic/apm/api/TransactionImpl.java index d6edd6a5f6..da0ce5bfe5 100644 --- a/apm-agent-api/src/main/java/co/elastic/apm/api/TransactionImpl.java +++ b/apm-agent-api/src/main/java/co/elastic/apm/api/TransactionImpl.java @@ -42,6 +42,13 @@ public Transaction setName(String name) { return this; } + @Nonnull + @Override + public Transaction setFrameworkName(String frameworkName) { + // co.elastic.apm.agent.pluginapi.TransactionInstrumentation$SetFrameworkNameInstrumentation + return this; + } + @Nonnull @Override public Transaction setType(String type) { diff --git a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java index c63059dee2..a166f2270d 100644 --- a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java +++ b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java @@ -53,6 +53,20 @@ public ElementMatcher getMethodMatcher() { return methodMatcher; } + public static class SetFrameworkNameInstrumentation extends TransactionInstrumentation { + public SetFrameworkNameInstrumentation() { + super(named("setFrameworkName")); + } + + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + public static void setFrameworkName(@Advice.FieldValue(value = "span", typing = Assigner.Typing.DYNAMIC) Object transaction, + @Advice.Argument(0) String frameworkName) { + if (transaction instanceof Transaction) { + ((Transaction) transaction).setFrameworkName(frameworkName); + } + } + } + public static class SetUserInstrumentation extends TransactionInstrumentation { public SetUserInstrumentation() { super(named("setUser")); diff --git a/apm-agent-plugins/apm-api-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation b/apm-agent-plugins/apm-api-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation index f0503805d6..161e9bceca 100644 --- a/apm-agent-plugins/apm-api-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation +++ b/apm-agent-plugins/apm-api-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation @@ -3,6 +3,7 @@ co.elastic.apm.agent.pluginapi.ElasticApmApiInstrumentation$StartTransactionWith co.elastic.apm.agent.pluginapi.ElasticApmApiInstrumentation$CurrentTransactionInstrumentation co.elastic.apm.agent.pluginapi.ElasticApmApiInstrumentation$CurrentSpanInstrumentation co.elastic.apm.agent.pluginapi.ElasticApmApiInstrumentation$CaptureExceptionInstrumentation +co.elastic.apm.agent.pluginapi.TransactionInstrumentation$SetFrameworkNameInstrumentation co.elastic.apm.agent.pluginapi.TransactionInstrumentation$SetUserInstrumentation co.elastic.apm.agent.pluginapi.TransactionInstrumentation$EnsureParentIdInstrumentation co.elastic.apm.agent.pluginapi.TransactionInstrumentation$SetResultInstrumentation 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 6deef7c721..ae8a029961 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 @@ -57,6 +57,13 @@ void testFrameworkName() { assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo("API"); } + @Test + void testSetFrameworkName() { + transaction.setFrameworkName("foo"); + endTransaction(); + assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo("foo"); + } + @Test void testSetType() { transaction.setType("foo"); @@ -129,7 +136,6 @@ private void checkResult(String expected) { } - @Test void testChaining() { int randomInt = random.nextInt(); From ebdf1a700e47d22fa2c5c2cf9e30a3a608029cb3 Mon Sep 17 00:00:00 2001 From: Tobias Stadler Date: Sat, 10 Jul 2021 10:55:28 +0200 Subject: [PATCH 2/7] Added #1909 to the changelog --- CHANGELOG.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index b7f59340c3..44b46728df 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -57,6 +57,7 @@ Elasticsearch's REST client - {pull}1883[#1883] * Added the ability to manually create exit spans, which will result with the auto creation of service nodes in the service map and downstream service in the dependencies table - {pull}1898[#1898] * Basic support for com.sun.net.httpserver.HttpServer - {pull}1854[#1854] +* Added support for setting the framework using the public api (#1908) - {pull}1909[#1909] [float] ===== Bug fixes From 4a59e60b7050f066406351b585e1df70c014b938 Mon Sep 17 00:00:00 2001 From: Tobias Stadler Date: Mon, 12 Jul 2021 08:35:18 +0200 Subject: [PATCH 3/7] Added setFrameworkName to the documentation --- docs/public-api.asciidoc | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/public-api.asciidoc b/docs/public-api.asciidoc index 7a275eff84..2843d7dfe1 100644 --- a/docs/public-api.asciidoc +++ b/docs/public-api.asciidoc @@ -311,6 +311,23 @@ transaction.setType(Transaction.TYPE_REQUEST); * `type`: The type of the transaction +[float] +[[api-transaction-set-framework-name]] +==== `Transaction setFrameworkName(String frameworkName)` added[1.25.0] +Override the name of the framework for the current transaction. +For supported frameworks, +the framework name is determined automatically, +and can be overridden using this function. + +Example: + +[source,java] +---- +transaction.setFrameworkName("My Framework"); +---- + +* `frameworkName`: The name of the framework + [float] [[api-transaction-add-tag]] ==== `Transaction setLabel(String key, value)` added[1.5.0 as `addLabel`,Number and boolean labels require APM Server 6.7] From e0cd817ec0d6f6223dd652742a35efc66304e32e Mon Sep 17 00:00:00 2001 From: Tobias Stadler Date: Mon, 12 Jul 2021 08:49:29 +0200 Subject: [PATCH 4/7] Make sure that a plugin does not override the framework name set by the user --- .../agent/impl/transaction/Transaction.java | 10 +++++++ .../pluginapi/TransactionInstrumentation.java | 2 +- .../TransactionInstrumentationTest.java | 30 +++++++++++++++++-- .../apm/api/AbstractSpanImplAccessor.java | 26 ++++++++++++++++ 4 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AbstractSpanImplAccessor.java diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java index 236f5958ec..84d0723a59 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java @@ -93,6 +93,8 @@ protected Labels.Mutable initialValue() { @Nullable private String frameworkName; + private boolean frameworkNameSetByUser; + @Nullable private String frameworkVersion; @@ -319,7 +321,15 @@ protected void recycle() { } public void setFrameworkName(@Nullable String frameworkName) { + if (frameworkNameSetByUser) { + return; + } + this.frameworkName = frameworkName; + } + + public void setUserFrameworkName(@Nullable String frameworkName) { this.frameworkName = frameworkName; + this.frameworkNameSetByUser = true; } @Nullable diff --git a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java index a166f2270d..d1aea56b78 100644 --- a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java +++ b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java @@ -62,7 +62,7 @@ public SetFrameworkNameInstrumentation() { public static void setFrameworkName(@Advice.FieldValue(value = "span", typing = Assigner.Typing.DYNAMIC) Object transaction, @Advice.Argument(0) String frameworkName) { if (transaction instanceof Transaction) { - ((Transaction) transaction).setFrameworkName(frameworkName); + ((Transaction) transaction).setUserFrameworkName(frameworkName); } } } 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 ae8a029961..3b70ebde09 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 @@ -20,6 +20,7 @@ import co.elastic.apm.AbstractApiTest; import co.elastic.apm.agent.impl.TracerInternalApiUtils; +import co.elastic.apm.api.AbstractSpanImplAccessor; import co.elastic.apm.api.ElasticApm; import co.elastic.apm.api.Outcome; import co.elastic.apm.api.Span; @@ -27,6 +28,8 @@ import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import java.security.SecureRandom; @@ -57,13 +60,36 @@ void testFrameworkName() { assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo("API"); } - @Test - void testSetFrameworkName() { + static String[] frameworkNames() { + return new String[]{null, "", "bar"}; + } + + @ParameterizedTest + @MethodSource("frameworkNames") + void testSetUserFrameworkNameBeforeItIsSetByAPlugin(String frameworkName) { + AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName(frameworkName); transaction.setFrameworkName("foo"); endTransaction(); assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo("foo"); } + @ParameterizedTest + @MethodSource("frameworkNames") + void testSetUserFrameworkNameAfterItIsSetByAPlugin(String frameworkName) { + transaction.setFrameworkName("foo"); + AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName(frameworkName); + endTransaction(); + assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo("foo"); + } + + @ParameterizedTest + @MethodSource("frameworkNames") + void testSetUserFrameworkName(String frameworkName) { + transaction.setFrameworkName(frameworkName); + endTransaction(); + assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo(frameworkName); + } + @Test void testSetType() { transaction.setType("foo"); diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AbstractSpanImplAccessor.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AbstractSpanImplAccessor.java new file mode 100644 index 0000000000..ada066ad77 --- /dev/null +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AbstractSpanImplAccessor.java @@ -0,0 +1,26 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package co.elastic.apm.api; + +public class AbstractSpanImplAccessor { + + public static co.elastic.apm.agent.impl.transaction.Transaction accessTransaction(Transaction t) { + return (co.elastic.apm.agent.impl.transaction.Transaction) ((TransactionImpl) t).span; + } +} From aa625e3bd71ff8bec86e6c64fb54a976a04f125f Mon Sep 17 00:00:00 2001 From: Tobias Stadler Date: Mon, 12 Jul 2021 10:04:27 +0200 Subject: [PATCH 5/7] Clarify setting framework name to null/empty string as suggested by @eyalkoren --- .../src/main/java/co/elastic/apm/api/Transaction.java | 3 +++ docs/public-api.asciidoc | 1 + 2 files changed, 4 insertions(+) diff --git a/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java b/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java index 1931119da9..bbfc6a3046 100644 --- a/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java +++ b/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java @@ -64,6 +64,9 @@ public interface Transaction extends Span { * the framework name is determined automatically, * and can be overridden using this function. *

+ *

+ * null or the empty string will make the agent omit this field. + *

* * @param frameworkName The name of the framework */ diff --git a/docs/public-api.asciidoc b/docs/public-api.asciidoc index 2843d7dfe1..4940b8f876 100644 --- a/docs/public-api.asciidoc +++ b/docs/public-api.asciidoc @@ -318,6 +318,7 @@ Override the name of the framework for the current transaction. For supported frameworks, the framework name is determined automatically, and can be overridden using this function. +`null` or the empty string will make the agent omit this field. Example: From 9bd523e1af30bfdbde9de75be84b026263cc2518 Mon Sep 17 00:00:00 2001 From: eyalkoren <41850454+eyalkoren@users.noreply.github.com> Date: Tue, 13 Jul 2021 11:22:59 +0300 Subject: [PATCH 6/7] Fix serialization of empty value, docs changes and some added tests --- .../java/co/elastic/apm/api/Transaction.java | 16 +++++--------- .../agent/impl/transaction/Transaction.java | 9 ++++---- .../serialize/DslJsonSerializerTest.java | 18 ++++++++++++++++ .../TransactionInstrumentationTest.java | 21 ++++++++++++++----- docs/public-api.asciidoc | 2 +- 5 files changed, 45 insertions(+), 21 deletions(-) diff --git a/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java b/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java index bbfc6a3046..a4a4f1d4ee 100644 --- a/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java +++ b/apm-agent-api/src/main/java/co/elastic/apm/api/Transaction.java @@ -58,17 +58,11 @@ public interface Transaction extends Span { Transaction setType(String type); /** - * Override the name of the framework for the current transaction. - *

- * For supported frameworks, - * the framework name is determined automatically, - * and can be overridden using this function. - *

- *

- * null or the empty string will make the agent omit this field. - *

- * - * @param frameworkName The name of the framework + * Provides a way to manually set the {@code service.framework.name} field. + * Any value set through this method will take precedence over automatically-set value for supported frameworks. + + * @param frameworkName The name of the framework. {@code null} and empty values will cause the exclusion of the + * framework name from the transaction context. */ @Nonnull Transaction setFrameworkName(String frameworkName); diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java index 84d0723a59..3c81358032 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java @@ -31,10 +31,7 @@ import org.HdrHistogram.WriterReaderPhaser; import javax.annotation.Nullable; -import java.util.Arrays; -import java.util.HashSet; import java.util.List; -import java.util.Set; /** * Data captured by an agent representing an event occurring in a monitored service @@ -328,7 +325,11 @@ public void setFrameworkName(@Nullable String frameworkName) { } public void setUserFrameworkName(@Nullable String frameworkName) { - this.frameworkName = frameworkName; + if (frameworkName != null && frameworkName.isEmpty()) { + this.frameworkName = null; + } else { + this.frameworkName = frameworkName; + } this.frameworkNameSetByUser = true; } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java index 4939e40797..d3b6180879 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java @@ -395,6 +395,24 @@ void testSpanDestinationContextSerialization() { assertThat(service.get("type").textValue()).isEqualTo(""); } + @Test + void testTransactionNullFrameworkNameSerialization() { + Transaction transaction = new Transaction(MockTracer.create()); + transaction.getTraceContext().setServiceName("service-name"); + transaction.setUserFrameworkName(null); + JsonNode transactionJson = readJsonString(serializer.toJsonString(transaction)); + assertThat(transactionJson.get("context").get("service").get("framework")).isNull(); + } + + @Test + void testTransactionEmptyFrameworkNameSerialization() { + Transaction transaction = new Transaction(MockTracer.create()); + transaction.getTraceContext().setServiceName("service-name"); + transaction.setUserFrameworkName(""); + JsonNode transactionJson = readJsonString(serializer.toJsonString(transaction)); + assertThat(transactionJson.get("context").get("service").get("framework")).isNull(); + } + @Test void testSpanInvalidDestinationSerialization() { Span span = new Span(MockTracer.create()); 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 3b70ebde09..688a48a58e 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 @@ -31,6 +31,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import javax.annotation.Nullable; import java.security.SecureRandom; import static org.assertj.core.api.Assertions.assertThat; @@ -66,25 +67,35 @@ static String[] frameworkNames() { @ParameterizedTest @MethodSource("frameworkNames") - void testSetUserFrameworkNameBeforeItIsSetByAPlugin(String frameworkName) { - AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName(frameworkName); + void testSetUserFrameworkValidNameBeforeSetByInternalAPI(@Nullable String frameworkName) { transaction.setFrameworkName("foo"); + AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName(frameworkName); endTransaction(); assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo("foo"); } @ParameterizedTest @MethodSource("frameworkNames") - void testSetUserFrameworkNameAfterItIsSetByAPlugin(String frameworkName) { - transaction.setFrameworkName("foo"); + void testSetUserFrameworkValidNameAfterSetByInternalAPI(@Nullable String frameworkName) { AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName(frameworkName); + transaction.setFrameworkName("foo"); endTransaction(); assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo("foo"); } @ParameterizedTest @MethodSource("frameworkNames") - void testSetUserFrameworkName(String frameworkName) { + void testSetUserFrameworkAnyNameBeforeSetByInternalAPI(@Nullable String frameworkName) { + transaction.setFrameworkName(frameworkName); + AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName("foo"); + endTransaction(); + assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo(frameworkName); + } + + @ParameterizedTest + @MethodSource("frameworkNames") + void testSetUserFrameworkAnyNameAfterSetByInternalAPI(@Nullable String frameworkName) { + AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName("foo"); transaction.setFrameworkName(frameworkName); endTransaction(); assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo(frameworkName); diff --git a/docs/public-api.asciidoc b/docs/public-api.asciidoc index 4940b8f876..2861f1c11e 100644 --- a/docs/public-api.asciidoc +++ b/docs/public-api.asciidoc @@ -314,7 +314,7 @@ transaction.setType(Transaction.TYPE_REQUEST); [float] [[api-transaction-set-framework-name]] ==== `Transaction setFrameworkName(String frameworkName)` added[1.25.0] -Override the name of the framework for the current transaction. +Provides a way to manually set the `service.framework.name` field. For supported frameworks, the framework name is determined automatically, and can be overridden using this function. From 8dec8e545d800ada695392286df0d21e9220499c Mon Sep 17 00:00:00 2001 From: eyalkoren <41850454+eyalkoren@users.noreply.github.com> Date: Tue, 13 Jul 2021 12:32:28 +0300 Subject: [PATCH 7/7] Fix tests --- .../TransactionInstrumentationTest.java | 38 +++++++++---------- 1 file changed, 18 insertions(+), 20 deletions(-) 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 688a48a58e..92e584d083 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 @@ -61,44 +61,42 @@ void testFrameworkName() { assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo("API"); } - static String[] frameworkNames() { - return new String[]{null, "", "bar"}; - } - - @ParameterizedTest - @MethodSource("frameworkNames") - void testSetUserFrameworkValidNameBeforeSetByInternalAPI(@Nullable String frameworkName) { + @Test + void testSetUserFrameworkValidNameBeforeSetByInternalAPI() { transaction.setFrameworkName("foo"); - AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName(frameworkName); + AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName("bar"); endTransaction(); assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo("foo"); } - @ParameterizedTest - @MethodSource("frameworkNames") - void testSetUserFrameworkValidNameAfterSetByInternalAPI(@Nullable String frameworkName) { - AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName(frameworkName); + @Test + void testSetUserFrameworkValidNameAfterSetByInternalAPI() { + AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName("bar"); transaction.setFrameworkName("foo"); endTransaction(); assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo("foo"); } + static String[] invalidFrameworkNames() { + return new String[]{null, ""}; + } + @ParameterizedTest - @MethodSource("frameworkNames") - void testSetUserFrameworkAnyNameBeforeSetByInternalAPI(@Nullable String frameworkName) { + @MethodSource("invalidFrameworkNames") + void testSetUserFrameworkInvalidNameBeforeSetByInternalAPI(@Nullable String frameworkName) { transaction.setFrameworkName(frameworkName); - AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName("foo"); + AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName("bar"); endTransaction(); - assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo(frameworkName); + assertThat(reporter.getFirstTransaction().getFrameworkName()).isNull(); } @ParameterizedTest - @MethodSource("frameworkNames") - void testSetUserFrameworkAnyNameAfterSetByInternalAPI(@Nullable String frameworkName) { - AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName("foo"); + @MethodSource("invalidFrameworkNames") + void testSetUserFrameworkInvalidNameAfterSetByInternalAPI(@Nullable String frameworkName) { + AbstractSpanImplAccessor.accessTransaction(transaction).setFrameworkName("bar"); transaction.setFrameworkName(frameworkName); endTransaction(); - assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo(frameworkName); + assertThat(reporter.getFirstTransaction().getFrameworkName()).isNull(); } @Test