Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NR-64061 Add user tracking to transactions and errors via the "enduser.id" attribute #1188

Merged
merged 17 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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
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
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
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
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
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);
}

}
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
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) {
obenkenobi marked this conversation as resolved.
Show resolved Hide resolved
}

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