Skip to content

Commit

Permalink
Merge pull request #1188 from newrelic/NR-64061-error-userid-attr
Browse files Browse the repository at this point in the history 
NR-64061 Add user tracking to transactions and errors via the "enduser.id" attribute
  • Loading branch information
@obenkenobi
obenkenobi authored Mar 28, 2023
2 parents b3e5725 + 9e0526b commit a7aacd4
Show file tree
Hide file tree
Showing 11 changed files with 105 additions and 21 deletions.
5 changes: 5 additions & 0 deletions agent-bridge/src/main/java/com/newrelic/agent/bridge/NoOpPublicApi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ public void addCustomParameters(Map<String, Object> params) {

}

@Override
public void setUserId(String userId) {

}

@Override
public void setTransactionName(String category, String name) {

Expand Down
7 changes: 7 additions & 0 deletions agent-bridge/src/main/java/com/newrelic/agent/bridge/PublicApi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,13 @@ public interface PublicApi {
*/
void addCustomParameters(Map<String, Object> params);

/**
* Sets the user ID for the current transaction by adding the "enduser.id" agent attribute. It is reported in errors and transaction traces.
*
* @param userId The user ID to report. If it is a null or blank String, the "enduser.id" agent attribute will not be included in the current transaction and any associated errors.
*/
void setUserId(String userId);

/**
* Set the name of the current transaction.
*
Expand Down
10 changes: 10 additions & 0 deletions agent-bridge/src/main/java/com/newrelic/api/agent/NewRelic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,16 @@ public static void addCustomParameters(Map<String, Object> params) {
AgentBridge.publicApi.addCustomParameters(params);
}


/**
* Sets the user ID for the current transaction by adding the "enduser.id" agent attribute. It is reported in errors and transaction traces.
*
* @param userId The user ID to report. If it is a null or blank String, the "enduser.id" agent attribute will not be included in the current transaction and any associated errors.
*/
public static void setUserId(String userId) {
AgentBridge.publicApi.setUserId(userId);
}

/**
* Set the name of the current transaction.
*
Expand Down
21 changes: 21 additions & 0 deletions functional_test/src/test/java/test/newrelic/test/agent/api/ApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,27 @@ private void runTestAddCustomBoolParameter() {
Assert.assertEquals(true, Transaction.getTransaction().getUserAttributes().get("bool"));
}


@Test
public void testSetUserId() throws Exception {
try {
runTestSetUserId();
} finally {
Transaction.clearTransaction();
}
}


@Trace(dispatcher = true)
private void runTestSetUserId() {
NewRelic.setUserId("hello");
Assert.assertEquals("hello", Transaction.getTransaction().getAgentAttributes().get("enduser.id"));
NewRelic.setUserId("");
Assert.assertFalse("Agent attributes shouldn't have user ID", Transaction.getTransaction().getAgentAttributes().containsKey("enduser.id"));
NewRelic.setUserId(null);
Assert.assertFalse("Agent attributes shouldn't have user ID", Transaction.getTransaction().getAgentAttributes().containsKey("enduser.id"));
}

@Test
public void testIgnoreApdexNotSet() {
Transaction tx = Transaction.getTransaction();
Expand Down
2 changes: 2 additions & 0 deletions instrumentation/servlet-user-5.0/src/main/java/jakarta/servlet/Filter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import java.security.Principal;

import com.newrelic.api.agent.NewRelic;
import jakarta.servlet.http.HttpServletRequest;

import com.newrelic.agent.bridge.AgentBridge;
Expand All @@ -27,6 +28,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
Principal principal = ((HttpServletRequest) request).getUserPrincipal();
if (principal != null) {
AgentBridge.getAgent().getTransaction().getAgentAttributes().put("user", principal.getName());
NewRelic.setUserId(principal.getName());
}
}

Expand Down
9 changes: 5 additions & 4 deletions instrumentation/servlet-user-5.0/src/main/java/jakarta/servlet/Servlet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@

package jakarta.servlet;

import java.security.Principal;

import jakarta.servlet.http.HttpServletRequest;

import com.newrelic.agent.bridge.AgentBridge;
import com.newrelic.api.agent.NewRelic;
import com.newrelic.api.agent.Trace;
import com.newrelic.api.agent.weaver.MatchType;
import com.newrelic.api.agent.weaver.Weave;
import com.newrelic.api.agent.weaver.Weaver;
import jakarta.servlet.http.HttpServletRequest;

import java.security.Principal;

@Weave(type = MatchType.Interface)
public abstract class Servlet {
Expand All @@ -27,6 +27,7 @@ public void service(ServletRequest request, ServletResponse response) {
Principal principal = ((HttpServletRequest) request).getUserPrincipal();
if (principal != null) {
AgentBridge.getAgent().getTransaction().getAgentAttributes().put("user", principal.getName());
NewRelic.setUserId(principal.getName());
}
}

Expand Down
2 changes: 2 additions & 0 deletions instrumentation/servlet-user/src/main/java/javax/servlet/Filter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import javax.servlet.http.HttpServletRequest;

import com.newrelic.agent.bridge.AgentBridge;
import com.newrelic.api.agent.NewRelic;
import com.newrelic.api.agent.Trace;
import com.newrelic.api.agent.weaver.MatchType;
import com.newrelic.api.agent.weaver.Weave;
Expand All @@ -27,6 +28,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
Principal principal = ((HttpServletRequest) request).getUserPrincipal();
if (principal != null) {
AgentBridge.getAgent().getTransaction().getAgentAttributes().put("user", principal.getName());
NewRelic.setUserId(principal.getName());
}
}

Expand Down
9 changes: 5 additions & 4 deletions instrumentation/servlet-user/src/main/java/javax/servlet/Servlet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@

package javax.servlet;

import java.security.Principal;

import javax.servlet.http.HttpServletRequest;

import com.newrelic.agent.bridge.AgentBridge;
import com.newrelic.api.agent.NewRelic;
import com.newrelic.api.agent.Trace;
import com.newrelic.api.agent.weaver.MatchType;
import com.newrelic.api.agent.weaver.Weave;
import com.newrelic.api.agent.weaver.Weaver;

import javax.servlet.http.HttpServletRequest;
import java.security.Principal;

@Weave(type = MatchType.Interface)
public abstract class Servlet {

Expand All @@ -27,6 +27,7 @@ public void service(ServletRequest request, ServletResponse response) {
Principal principal = ((HttpServletRequest) request).getUserPrincipal();
if (principal != null) {
AgentBridge.getAgent().getTransaction().getAgentAttributes().put("user", principal.getName());
NewRelic.setUserId(principal.getName());
}
}

Expand Down
4 changes: 4 additions & 0 deletions newrelic-agent/src/main/java/com/newrelic/agent/attributes/AgentAttributeSender.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,8 @@ protected Map<String, Object> getAttributeMap() {
}
}

public void removeAttribute(String key) {
getAttributeMap().remove(key);
}

}
48 changes: 35 additions & 13 deletions newrelic-agent/src/main/java/com/newrelic/api/agent/NewRelicApiImplementation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.newrelic.agent.Agent;
import com.newrelic.agent.MetricNames;
import com.newrelic.agent.Transaction;
import com.newrelic.agent.attributes.AgentAttributeSender;
import com.newrelic.agent.attributes.AttributeSender;
import com.newrelic.agent.attributes.CustomAttributeSender;
import com.newrelic.agent.bridge.AgentBridge;
Expand All @@ -36,14 +37,17 @@
* DO NOT INVOKE THIS CLASS DIRECTLY. Use {@link com.newrelic.api.agent.NewRelic}.
*/
public class NewRelicApiImplementation implements PublicApi {
private final AttributeSender attributeSender;
private final AttributeSender customAttributeSender;

public NewRelicApiImplementation(AttributeSender sender) {
attributeSender = sender;
private final AgentAttributeSender agentAttributeSender;

public NewRelicApiImplementation(AttributeSender customSender, AgentAttributeSender agentSender) {
customAttributeSender = customSender;
agentAttributeSender = agentSender;
}

public NewRelicApiImplementation() {
this(new CustomAttributeSender());
this(new CustomAttributeSender(), new AgentAttributeSender());
}

// ************************** Error collector ***********************************//
Expand Down Expand Up @@ -109,7 +113,7 @@ public void noticeError(Throwable throwable, boolean expected) {

public void noticeError(Throwable throwable, Map<String, ?> params, boolean expected) {
try {
ServiceFactory.getRPMService().getErrorService().reportException(throwable, filterErrorAtts(params, attributeSender), expected);
ServiceFactory.getRPMService().getErrorService().reportException(throwable, filterErrorAtts(params, customAttributeSender), expected);

MetricNames.recordApiSupportabilityMetric(MetricNames.SUPPORTABILITY_API_NOTICE_ERROR);
//SUPPORTABILITY_API_EXPECTED_ERROR_API_THROWABLE metric is intended to be recorded independent of whether
Expand All @@ -132,7 +136,7 @@ public void noticeError(Throwable throwable, Map<String, ?> params, boolean expe
@Override
public void noticeError(String message, Map<String, ?> params, boolean expected) {
try {
ServiceFactory.getRPMService().getErrorService().reportError(message, filterErrorAtts(params, attributeSender), expected);
ServiceFactory.getRPMService().getErrorService().reportError(message, filterErrorAtts(params, customAttributeSender), expected);

MetricNames.recordApiSupportabilityMetric(MetricNames.SUPPORTABILITY_API_NOTICE_ERROR);
if (expected) {
Expand Down Expand Up @@ -250,7 +254,7 @@ private static int getNumberOfErrorAttsLeft() {
*/
@Override
public void addCustomParameter(String key, String value) {
attributeSender.addAttribute(key, value, "addCustomParameter");
customAttributeSender.addAttribute(key, value, "addCustomParameter");
}

/**
Expand All @@ -261,7 +265,7 @@ public void addCustomParameter(String key, String value) {
*/
@Override
public void addCustomParameter(String key, Number value) {
attributeSender.addAttribute(key, value, "addCustomParameter");
customAttributeSender.addAttribute(key, value, "addCustomParameter");
}

/**
Expand All @@ -272,7 +276,7 @@ public void addCustomParameter(String key, Number value) {
*/
@Override
public void addCustomParameter(String key, boolean value) {
attributeSender.addAttribute(key, value, "addCustomParameter");
customAttributeSender.addAttribute(key, value, "addCustomParameter");
}

/**
Expand All @@ -282,7 +286,25 @@ public void addCustomParameter(String key, boolean value) {
*/
@Override
public void addCustomParameters(Map<String, Object> params) {
attributeSender.addAttributes(params, "addCustomParameters");
customAttributeSender.addAttributes(params, "addCustomParameters");
}

/**
* Sets the user ID for the current transaction by adding the "enduser.id" agent attribute. It is reported in errors and transaction traces.
*
* @param userId The user ID to report. If it is a null or blank String, the "enduser.id" agent attribute will not be included in the current transaction and any associated errors.
*/
@Override
public void setUserId(String userId) {
final String attributeKey = "enduser.id";
final String methodCalled = "setUserId";
// Ignore null and empty strings
if (userId == null || userId.trim().isEmpty()) {
Agent.LOG.log(Level.FINER, "Will not include the {0} attribute because {1} was invoked with a null or blank value", attributeKey, methodCalled);
agentAttributeSender.removeAttribute(attributeKey);
return;
}
agentAttributeSender.addAttribute(attributeKey, userId, methodCalled);
}

/**
Expand Down Expand Up @@ -530,7 +552,7 @@ public void setUserName(String name) {
Agent.LOG.finer(msg);
}
MetricNames.recordApiSupportabilityMetric(MetricNames.SUPPORTABILITY_API_SET_USER_NAME);
attributeSender.addAttribute("user", name, "setUserName");
customAttributeSender.addAttribute("user", name, "setUserName");
}

/**
Expand All @@ -553,7 +575,7 @@ public void setAccountName(String name) {
String msg = MessageFormat.format("Attempting to set account name to \"{0}\" in NewRelic API", name);
Agent.LOG.finer(msg);
}
attributeSender.addAttribute("account", name, "setAccountName");
customAttributeSender.addAttribute("account", name, "setAccountName");
MetricNames.recordApiSupportabilityMetric(MetricNames.SUPPORTABILITY_API_SET_ACCOUNT_NAME);
}
}
Expand All @@ -579,7 +601,7 @@ public void setProductName(String name) {
Agent.LOG.finer(msg);
}
MetricNames.recordApiSupportabilityMetric(MetricNames.SUPPORTABILITY_API_SET_PRODUCT_NAME);
attributeSender.addAttribute("product", name, "setProductName");
customAttributeSender.addAttribute("product", name, "setProductName");
}
}

Expand Down
9 changes: 9 additions & 0 deletions newrelic-api/src/main/java/com/newrelic/api/agent/NewRelic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,15 @@ public static void addCustomParameter(String key, boolean value) {
public static void addCustomParameters(Map<String, Object> params) {
}

/**
* Sets the user ID for the current transaction by adding the "enduser.id" agent attribute. It is reported in errors and transaction traces.
*
* @param userId The user ID to report. If it is a null or blank String, the "enduser.id" agent attribute will not be included in the current transaction and any associated errors.
* @since 8.1.0
*/
public static void setUserId(String userId) {
}

/**
* Set the name of the current transaction.
*
Expand Down

0 comments on commit a7aacd4

Please sign in to comment.