Skip to content

Commit

Permalink
Merge branch 'main' into stall-detection-duplicate
Browse files Browse the repository at this point in the history
  • Loading branch information
@jtduffy
jtduffy authored Oct 9, 2023
2 parents 0412d2a + c94127d commit 9721e6e
Show file tree
Hide file tree
Showing 30 changed files with 939 additions and 704 deletions.
27 changes: 23 additions & 4 deletions agent-bridge/src/main/java/com/newrelic/agent/bridge/logging/AppLoggingUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
*/
package com.newrelic.agent.bridge.logging;

import com.newrelic.agent.bridge.logging.LogAttributeKey;
import com.newrelic.agent.bridge.logging.LogAttributeType;
import com.newrelic.api.agent.NewRelic;

import java.io.UnsupportedEncodingException;
Expand Down Expand Up @@ -54,11 +52,32 @@ public class AppLoggingUtils {
* @return agent linking metadata string blob
*/
public static String getLinkingMetadataBlob() {
Map<String, String> agentLinkingMetadata = NewRelic.getAgent().getLinkingMetadata();
return constructLinkingMetadataBlob(NewRelic.getAgent().getLinkingMetadata());
}

/**
* Gets a String representing the agent linking metadata in blob format:
* NR-LINKING|entity.guid|hostname|trace.id|span.id|entity.name|
*
* @param agentLinkingMetadata map of linking metadata
* @return agent linking metadata string blob
*/
public static String getLinkingMetadataBlobFromMap(Map<String, String> agentLinkingMetadata) {
return constructLinkingMetadataBlob(agentLinkingMetadata);
}

/**
* Constructs a String representing the agent linking metadata in blob format:
* NR-LINKING|entity.guid|hostname|trace.id|span.id|entity.name|
*
* @param agentLinkingMetadata map of linking metadata
* @return agent linking metadata string blob
*/
private static String constructLinkingMetadataBlob(Map<String, String> agentLinkingMetadata) {
StringBuilder blob = new StringBuilder();
blob.append(" ").append(BLOB_PREFIX).append(BLOB_DELIMITER);

if (agentLinkingMetadata != null && agentLinkingMetadata.size() > 0) {
if (agentLinkingMetadata != null && !agentLinkingMetadata.isEmpty()) {
appendAttributeToBlob(agentLinkingMetadata.get(ENTITY_GUID), blob);
appendAttributeToBlob(agentLinkingMetadata.get(HOSTNAME), blob);
appendAttributeToBlob(agentLinkingMetadata.get(TRACE_ID), blob);
Expand Down
69 changes: 50 additions & 19 deletions ...src/test/java/com/newrelic/agent/instrumentation/pointcuts/net/HttpURLConnectionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ public class HttpURLConnectionTest {
static final TransactionDataList transactions = new TransactionDataList();
private static final String GET_OUTPUT_STREAM = "getOutputStream";
private static final String GET_INPUT_STREAM = "getInputStream";
private static final String GET_RESPONSE_CODE = "getResponseCode";
private static final String GET_RESPONSE_MSG = "getResponseMessage";
private static final String URL = "example.com";
private static final String REAL_HOST = "https://example.com";
private static final String FAKE_HOST = "http://www.thishostdoesnotexistbro.com"; // Better hope no one buys this domain...
Expand Down Expand Up @@ -95,7 +97,7 @@ public void outputStreamFirstTest() {
doOutputStreamFirstTest();
String scope = format("OtherTransaction/Custom/{0}/doOutputStreamFirstTest", TEST_CLASS);

verifyMetrics(URL, scope, true, GET_OUTPUT_STREAM);
verifyMetrics(URL, scope, false, GET_OUTPUT_STREAM);
}

@Trace(dispatcher = true)
Expand Down Expand Up @@ -130,7 +132,7 @@ public void connectFirstTest() {
doConnectFirstTest();
String scope = format("OtherTransaction/Custom/{0}/doConnectFirstTest", TEST_CLASS);

verifyMetrics(URL, scope, true, GET_OUTPUT_STREAM);
verifyMetrics(URL, scope, false, GET_OUTPUT_STREAM);
}

@Trace(dispatcher = true)
Expand Down Expand Up @@ -255,7 +257,7 @@ public void testHttpURLConnectionMetrics3() {
run3();
String scope = format("OtherTransaction/Custom/{0}/run3", TEST_CLASS);

verifyMetrics(URL, scope, true, GET_INPUT_STREAM);
verifyMetrics(URL, scope, true, GET_RESPONSE_CODE);
}

@Trace(dispatcher = true)
Expand Down Expand Up @@ -330,7 +332,7 @@ public void testHttpURLConnectionMetrics6() {
run6();
String scope = format("OtherTransaction/Custom/{0}/run6", TEST_CLASS);

verifyMetrics(URL, scope, true, GET_OUTPUT_STREAM);
verifyMetrics(URL, scope, false, GET_OUTPUT_STREAM);
}

@Trace(dispatcher = true)
Expand All @@ -355,7 +357,7 @@ public void testHttpURLConnectionMetrics7() {
run7();
String scope = format("OtherTransaction/Custom/{0}/run7", TEST_CLASS);

verifyMetrics(URL, scope, true, GET_OUTPUT_STREAM);
verifyMetrics(URL, scope, true, GET_RESPONSE_CODE);
}

@Trace(dispatcher = true)
Expand Down Expand Up @@ -383,7 +385,7 @@ public void testHttpURLConnectionMetrics8() {
run8();
String scope = format("OtherTransaction/Custom/{0}/run8", TEST_CLASS);

verifyMetrics(URL, scope, true, GET_OUTPUT_STREAM);
verifyMetrics(URL, scope, true, GET_RESPONSE_CODE);
}

@Trace(dispatcher = true)
Expand All @@ -410,7 +412,7 @@ public void testHttpURLConnectionMetrics9() {
run9();
String scope = format("OtherTransaction/Custom/{0}/run9", TEST_CLASS);

verifyMetrics(URL, scope, true, GET_INPUT_STREAM);
verifyMetrics(URL, scope, true, GET_RESPONSE_CODE);
}

@Trace(dispatcher = true)
Expand All @@ -430,6 +432,31 @@ private void run9() {
}
}

@Test
public void testHttpURLConnectionMetrics10() {
run10();
String scope = format("OtherTransaction/Custom/{0}/run10", TEST_CLASS);

verifyMetrics(URL, scope, true, GET_RESPONSE_MSG);
}

@Trace(dispatcher = true)
private void run10() {
HttpURLConnection connection = null;
try {
// Test 9: getResponseMessage()
connection = getHttpsExampleComConnection();
// GETs URL and reads response message (Network I/O)
System.out.println("Response message: " + connection.getResponseMessage());
} catch (Exception e) {
e.printStackTrace();
} finally {
if (connection != null) {
connection.disconnect();
}
}
}

@Test
public void testUnresolvedHost() throws Exception {
unresolvedHost();
Expand Down Expand Up @@ -475,7 +502,8 @@ private void verifyMetrics(String url, String scope, boolean reportedExternalCal
Set<String> metrics = AgentHelper.getMetrics();
// Scoped external metrics
String httpURLConnectionGetInputStreamMetric = format("External/{0}/HttpURLConnection/getInputStream", url);
String httpURLConnectionGetOutputStreamMetric = format("External/{0}/HttpURLConnection/getOutputStream", url);
String httpURLConnectionGetResponseCodeMetric = format("External/{0}/HttpURLConnection/getResponseCode", url);
String httpURLConnectionGetResponseMessageMetric = format("External/{0}/HttpURLConnection/getResponseMessage", url);
// Unscoped external metrics
String externalHostAllMetric = format("External/{0}/all", url);
String externalAllMetric = "External/all";
Expand All @@ -488,8 +516,7 @@ private void verifyMetrics(String url, String scope, boolean reportedExternalCal
scopedMetricCount = 1;
// One of these scoped metrics should be generated when an external call is reported
Assert.assertTrue(metrics.toString(),
metrics.contains(httpURLConnectionGetOutputStreamMetric) ||
metrics.contains(httpURLConnectionGetInputStreamMetric));
metrics.contains(format("External/{0}/HttpURLConnection/" + methodInExternalMetric, url)));

unscopedMetricCount = 3;
// All three of these unscoped metrics should be generated when an external call is reported
Expand All @@ -498,25 +525,27 @@ private void verifyMetrics(String url, String scope, boolean reportedExternalCal
Assert.assertTrue(metrics.toString(), metrics.contains(externalAllOtherMetric));
} else {
Assert.assertFalse(metrics.toString(),
metrics.contains(httpURLConnectionGetOutputStreamMetric) ||
metrics.contains(httpURLConnectionGetInputStreamMetric));
metrics.contains(format("External/{0}/HttpURLConnection/" + methodInExternalMetric, url)));

Assert.assertFalse(metrics.toString(), metrics.contains(externalHostAllMetric));
Assert.assertFalse(metrics.toString(), metrics.contains(externalAllMetric));
Assert.assertFalse(metrics.toString(), metrics.contains(externalAllOtherMetric));
}

Map<String, Integer> scopedMetricCounts = getMetricCounts(
MetricName.create(httpURLConnectionGetOutputStreamMetric, scope),
MetricName.create(httpURLConnectionGetInputStreamMetric, scope));
MetricName.create(httpURLConnectionGetInputStreamMetric, scope),
MetricName.create(httpURLConnectionGetResponseCodeMetric, scope),
MetricName.create(httpURLConnectionGetResponseMessageMetric, scope)
);

Map<String, Integer> unscopedMetricCounts = getMetricCounts(
MetricName.create(externalHostAllMetric),
MetricName.create(externalAllMetric),
MetricName.create(externalAllOtherMetric));

int actualHttpURLConnectionScopedMetricCount = scopedMetricCounts.get(httpURLConnectionGetOutputStreamMetric) +
scopedMetricCounts.get(httpURLConnectionGetInputStreamMetric);
int actualHttpURLConnectionScopedMetricCount = scopedMetricCounts.get(httpURLConnectionGetInputStreamMetric) +
scopedMetricCounts.get(httpURLConnectionGetResponseCodeMetric) +
scopedMetricCounts.get(httpURLConnectionGetResponseMessageMetric);

int actualHttpURLConnectionUnscopedMetricCount = unscopedMetricCounts.get(externalHostAllMetric) +
unscopedMetricCounts.get(externalAllMetric) +
Expand All @@ -525,15 +554,17 @@ private void verifyMetrics(String url, String scope, boolean reportedExternalCal
assertEquals(scopedMetricCount, actualHttpURLConnectionScopedMetricCount);

if (scopedMetricCount == 0) {
assertEquals(0, (int) scopedMetricCounts.get(httpURLConnectionGetOutputStreamMetric));
assertEquals(0, (int) scopedMetricCounts.get(httpURLConnectionGetInputStreamMetric));
} else {
if (methodInExternalMetric != null) {
if (methodInExternalMetric.equals(GET_INPUT_STREAM)) {
assertEquals(1, (int) scopedMetricCounts.get(httpURLConnectionGetInputStreamMetric));
}
if (methodInExternalMetric.equals(GET_OUTPUT_STREAM)) {
assertEquals(1, (int) scopedMetricCounts.get(httpURLConnectionGetOutputStreamMetric));
if (methodInExternalMetric.equals(GET_RESPONSE_CODE)) {
assertEquals(1, (int) scopedMetricCounts.get(httpURLConnectionGetResponseCodeMetric));
}
if (methodInExternalMetric.equals(GET_RESPONSE_MSG)) {
assertEquals(1, (int) scopedMetricCounts.get(httpURLConnectionGetResponseMessageMetric));
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions functional_test/src/test/java/test/newrelic/test/agent/HttpCommonsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,13 @@ public void httpURLConnection() throws Exception {
httpURLConnectionTx();

Set<String> metrics = AgentHelper.getMetrics();
assertTrue(metrics.toString(), metrics.contains("External/" + HOST + "/HttpURLConnection/getInputStream"));
assertTrue(metrics.toString(), metrics.contains("External/" + HOST + "/HttpURLConnection/getResponseCode"));

Map<String, Integer> metricCounts = getMetricCounts(
MetricName.create("External/" + HOST + "/HttpURLConnection/getInputStream",
MetricName.create("External/" + HOST + "/HttpURLConnection/getResponseCode",
"OtherTransaction/Custom/test.newrelic.test.agent.HttpCommonsTest/httpURLConnectionTx"));

assertEquals(1, (int) metricCounts.get("External/" + HOST + "/HttpURLConnection/getInputStream"));
assertEquals(1, (int) metricCounts.get("External/" + HOST + "/HttpURLConnection/getResponseCode"));
}

@Trace(dispatcher = true)
Expand Down
5 changes: 2 additions & 3 deletions instrumentation/apache-log4j-2/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ jar {

dependencies {
implementation(project(":agent-bridge"))
implementation("org.apache.logging.log4j:log4j-core:2.17.1")
implementation("org.apache.logging.log4j:log4j-core:2.20.0")
}

verifyInstrumentation {
passesOnly("org.apache.logging.log4j:log4j-core:[2.6,)")
excludeRegex '.*(alpha|beta|rc).*'
passesOnly("org.apache.logging.log4j:log4j-core:[2.6,3.0.0-alpha1)")
}

site {
Expand Down
25 changes: 24 additions & 1 deletion ...mentation/apache-log4j-2/src/main/java/com/nr/agent/instrumentation/log4j2/AgentUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.message.Message;
import org.apache.logging.log4j.util.ReadOnlyStringMap;

import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -128,4 +127,28 @@ private static int calculateInitialMapSize(Map<String, String> mdcPropertyMap) {
? mdcPropertyMap.size() + DEFAULT_NUM_OF_LOG_EVENT_ATTRIBUTES
: DEFAULT_NUM_OF_LOG_EVENT_ATTRIBUTES;
}

/**
* Checks pretty or compact JSON layout strings for a series of characters and returns the index of
* the characters or -1 if they were not found. This is used to find the log "message" substring
* so that the NR-LINKING metadata blob can be inserted when using local decorating with JsonLayout.
*
* @param writerString String representing JSON formatted log event
* @return positive int if index was found, else -1
*/
public static int getIndexToModifyJson(String writerString) {
return writerString.indexOf("\",", writerString.indexOf("message"));
}

/**
* Check if a valid match was found when calling String.indexOf.
* If index value is -1 then no valid match was found, a positive integer represents a valid index.
*
* @param indexToModifyJson int representing index returned by indexOf
* @return true if a valid index was found, else false
*/
public static boolean foundIndexToInsertLinkingMetadata(int indexToModifyJson) {
return indexToModifyJson != -1;
}

}
80 changes: 80 additions & 0 deletions .../apache-log4j-2/src/main/java/org/apache/logging/log4j/core/LogEvent_Instrumentation.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/*
*
* * Copyright 2023 New Relic Corporation. All rights reserved.
* * SPDX-License-Identifier: Apache-2.0
*
*/

package org.apache.logging.log4j.core;

import com.newrelic.api.agent.NewRelic;
import com.newrelic.api.agent.weaver.MatchType;
import com.newrelic.api.agent.weaver.NewField;
import com.newrelic.api.agent.weaver.Weave;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Marker;
import org.apache.logging.log4j.ThreadContext;
import org.apache.logging.log4j.core.impl.ThrowableProxy;
import org.apache.logging.log4j.core.time.Instant;
import org.apache.logging.log4j.message.Message;
import org.apache.logging.log4j.util.ReadOnlyStringMap;

import java.util.Map;

import static com.newrelic.agent.bridge.logging.AppLoggingUtils.isApplicationLoggingLocalDecoratingEnabled;

@Weave(originalName = "org.apache.logging.log4j.core.LogEvent", type = MatchType.Interface)
public abstract class LogEvent_Instrumentation {

/*
* In cases where the LogEvent is sent to an AsyncAppender, getLinkingMetadata would get called on a new thread and the trace.id and span.id
* would be missing. To work around this we save the linking metadata on the LogEvent on the thread where it was created and use it later.
*/
@NewField
public Map<String, String> agentLinkingMetadata = isApplicationLoggingLocalDecoratingEnabled() ? NewRelic.getAgent().getLinkingMetadata() : null;

public abstract LogEvent toImmutable();

@Deprecated
public abstract Map<String, String> getContextMap();

public abstract ReadOnlyStringMap getContextData();

public abstract ThreadContext.ContextStack getContextStack();

public abstract String getLoggerFqcn();

public abstract Level getLevel();

public abstract String getLoggerName();

public abstract Marker getMarker();

public abstract Message getMessage();

public abstract long getTimeMillis();

public abstract Instant getInstant();

public abstract StackTraceElement getSource();

public abstract String getThreadName();

public abstract long getThreadId();

public abstract int getThreadPriority();

public abstract Throwable getThrown();

public abstract ThrowableProxy getThrownProxy();

public abstract boolean isEndOfBatch();

public abstract boolean isIncludeLocation();

public abstract void setEndOfBatch(boolean endOfBatch);

public abstract void setIncludeLocation(boolean locationRequired);

public abstract long getNanoTime();
}
Loading

0 comments on commit 9721e6e

Please sign in to comment.