Skip to content

Commit

Permalink
Fix transaction name for non-sampled transactions
Browse files Browse the repository at this point in the history
  • Loading branch information
felixbarny committed Apr 17, 2019
1 parent f51f253 commit 53510f2
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 46 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
# next (1.7.0)
# next

## Features

## Bug Fixes
* Fixes transaction name for non-sampled transactions [#581](https://github.com/elastic/apm-agent-java/issues/581)

# 1.6.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package co.elastic.apm.agent.impl.transaction;

import co.elastic.apm.agent.impl.ElasticApmTracer;
import co.elastic.apm.agent.impl.context.AbstractContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -72,9 +73,6 @@ public StringBuilder getName() {
* Generic designation of a transaction in the scope of a single service (eg: 'GET /users/:id')
*/
public void setName(@Nullable String name) {
if (!isSampled()) {
return;
}
this.name.setLength(0);
this.name.append(name);
}
Expand Down Expand Up @@ -130,11 +128,25 @@ public Span createSpan(long epochMicros) {
return tracer.startSpan(this, epochMicros);
}

public abstract void addLabel(String key, String value);
public void addLabel(String key, String value) {
if (isSampled()) {
getContext().addLabel(key, value);
}
}

public void addLabel(String key, Number value) {
if (isSampled()) {
getContext().addLabel(key, value);
}
}

public abstract void addLabel(String key, Number value);
public void addLabel(String key, Boolean value) {
if (isSampled()) {
getContext().addLabel(key, value);
}
}

public abstract void addLabel(String key, Boolean value);
public abstract AbstractContext getContext();

protected void onStart() {
this.finished = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public <T> Span start(TraceContext.ChildContextCreator<T> childContextCreator, T
/**
* Any other arbitrary data captured by the agent, optionally provided by the user
*/
@Override
public SpanContext getContext() {
return context;
}
Expand Down Expand Up @@ -191,21 +192,6 @@ public void resetState() {
action = null;
}

@Override
public void addLabel(String key, String value) {
context.addLabel(key, value);
}

@Override
public void addLabel(String key, Number value) {
context.addLabel(key, value);
}

@Override
public void addLabel(String key, Boolean value) {
context.addLabel(key, value);
}

public void recycle() {
tracer.recycle(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public Transaction startNoop() {
* <p>
* Any arbitrary contextual information regarding the event, captured by the agent, optionally provided by the user
*/
@Override
public TransactionContext getContext() {
return context;
}
Expand All @@ -104,9 +105,6 @@ public TransactionContext getContextEnsureVisibility() {
}

public Transaction withName(@Nullable String name) {
if (!isSampled()) {
return this;
}
setName(name);
return this;
}
Expand All @@ -131,31 +129,10 @@ public String getResult() {
* The result of the transaction. HTTP status code for HTTP-related transactions.
*/
public Transaction withResult(@Nullable String result) {
if (!isSampled()) {
return this;
}
this.result = result;
return this;
}

@Override
public void addLabel(String key, String value) {
if (!isSampled()) {
return;
}
getContext().addLabel(key, value);
}

@Override
public void addLabel(String key, Number value) {
context.addLabel(key, value);
}

@Override
public void addLabel(String key, Boolean value) {
context.addLabel(key, value);
}

public void setUser(String id, String email, String username) {
if (!isSampled()) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import co.elastic.apm.agent.MockReporter;
import co.elastic.apm.agent.bci.ElasticApmAgent;
import co.elastic.apm.agent.configuration.CoreConfiguration;
import co.elastic.apm.agent.configuration.SpyConfiguration;
import co.elastic.apm.agent.impl.ElasticApmTracer;
import co.elastic.apm.agent.impl.ElasticApmTracerBuilder;
Expand All @@ -38,6 +39,7 @@
import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.core.Application;
import java.io.IOException;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -104,6 +106,17 @@ public void testJaxRsTransactionNameWithJaxrsAnnotationInheritance() {
assertThat(actualTransactions.get(2).getName().toString()).isEqualTo("ResourceWithPathOnAbstract#testMethod");
}

@Test
public void testJaxRsTransactionNameNonSampledTransactions() throws IOException {
config.getConfig(CoreConfiguration.class).getSampleRate().update(0.0, SpyConfiguration.CONFIG_SOURCE_NAME);

ElasticApmAgent.initInstrumentation(tracer, ByteBuddyAgent.install());

doRequest("test");
List<Transaction> actualTransactions = reporter.getTransactions();
assertThat(actualTransactions).hasSize(1);
assertThat(actualTransactions.get(0).getName().toString()).isEqualTo("ResourceWithPath#testMethod");
}

/**
* @return configuration for the jersey test server. Includes all resource classes in the co.elastic.apm.agent.jaxrs.resources package.
Expand Down

0 comments on commit 53510f2

Please sign in to comment.