Skip to content

Removing abs_path and adding module to serialized stack frames #659

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 12, 2019
Merged

Removing abs_path and adding module to serialized stack frames #659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 19 additions & 1 deletion apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,14 @@ public String toJsonString(final ErrorCapture error) {
return s;
}

public String toJsonString(final StackTraceElement stackTraceElement) {
jw.reset();
serializeStackTraceElement(stackTraceElement);
final String s = jw.toString();
jw.reset();
return s;
}

public String toString() {
return jw.toString();
}
Expand Down Expand Up @@ -656,10 +664,20 @@ private void serializeStackTraceElement(StackTraceElement stacktrace) {
writeField("function", stacktrace.getMethodName());
writeField("library_frame", isLibraryFrame(stacktrace.getClassName()));
writeField("lineno", stacktrace.getLineNumber());
writeLastField("abs_path", stacktrace.getClassName());
serializeStackFrameModule(stacktrace.getClassName());
jw.writeByte(OBJECT_END);
}

private void serializeStackFrameModule(final String fullyQualifiedClassName) {
Copy link
Member

@felixbarny felixbarny Jun 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using the Java 9 module or jar file name instead of the package name?

Copy link
Contributor Author

@eyalkoren eyalkoren Jun 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like using the package name aligns better with the majority of other agents (using sort of namespace for module).
@axw WDYT?

Copy link
Member

@axw axw Jun 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think the package name aligns better with the other agents.

  • In Python, it's the Python module, which is effectively a namespace. This may be where the field name originated. In Python, there could be multiple modules distributed together in one package (~jar).
  • In Go, it's the package path (again, ~namespace). Again, there could be multiple packages within one module, a relatively new concept to Go. Just to keep us on our toes, the names/relations are flipped from Python.
  • In Node.js, it's the node module name.
  • For Ruby, there's no reported module as it's not readily available.

The Jar/module name could be useful for linking to Code Search, but I would think it'd be more useful for now to report the package name than the jar, since a large project may have multiple packages with the classes/files of the same name.

writeFieldName("module");
replaceBuilder.setLength(0);
final int lastDotIndex = fullyQualifiedClassName.lastIndexOf('.');
if (lastDotIndex > 0) {
replaceBuilder.append(fullyQualifiedClassName, 0, lastDotIndex);
}
writeStringBuilderValue(replaceBuilder, jw);
}

private boolean isLibraryFrame(String className) {
for (String applicationPackage : stacktraceConfiguration.getApplicationPackages()) {
if (className.startsWith(applicationPackage)) {
Expand Down
15 changes: 13 additions & 2 deletions ...agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.SoftAssertions.assertSoftly;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;


class DslJsonSerializerTest {
Expand All @@ -61,7 +62,9 @@ class DslJsonSerializerTest {

@BeforeEach
void setUp() {
serializer = new DslJsonSerializer(mock(StacktraceConfiguration.class));
StacktraceConfiguration stacktraceConfiguration = mock(StacktraceConfiguration.class);
when(stacktraceConfiguration.getStackTraceLimit()).thenReturn(15);
serializer = new DslJsonSerializer(stacktraceConfiguration);
objectMapper = new ObjectMapper();
}

Expand Down Expand Up @@ -97,7 +100,15 @@ void testErrorSerialization() throws IOException {
assertThat(context.get("tags").get("foo").textValue()).isEqualTo("bar");
JsonNode exception = errorTree.get("exception");
assertThat(exception.get("message").textValue()).isEqualTo("test");
assertThat(exception.get("stacktrace")).isNotNull();
JsonNode stacktrace = exception.get("stacktrace");
assertThat(stacktrace).isNotNull();
assertThat(stacktrace).hasSize(15);
JsonNode stackTraceElement = stacktrace.get(0);
assertThat(stackTraceElement.get("filename")).isNotNull();
assertThat(stackTraceElement.get("function")).isNotNull();
assertThat(stackTraceElement.get("library_frame")).isNotNull();
assertThat(stackTraceElement.get("lineno")).isNotNull();
assertThat(stackTraceElement.get("module")).isNotNull();
assertThat(exception.get("type").textValue()).isEqualTo(Exception.class.getName());
assertThat(errorTree.get("transaction").get("sampled").booleanValue()).isTrue();
assertThat(errorTree.get("transaction").get("type").textValue()).isEqualTo("test-type");
Expand Down
47 changes: 41 additions & 6 deletions apm-agent-core/src/test/java/org/example/stacktrace/StacktraceSerializationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,17 @@ void setUp() throws IOException {
void fillStackTrace() {
assertThat(stacktrace).isNotEmpty();
// even though the stacktrace is captured within our tracer class, the first method should be getStackTrace
assertThat(stacktrace.get(0).get("abs_path").textValue()).doesNotStartWith("co.elastic");
assertThat(stacktrace.get(0).get("module").textValue()).doesNotStartWith("co.elastic");
assertThat(stacktrace.get(0).get("function").textValue()).isEqualTo("getStackTrace");
assertThat(stacktrace.stream().filter(st -> st.get("abs_path").textValue().endsWith("StacktraceSerializationTest"))).isNotEmpty();
assertThat(stacktrace.stream().filter(st -> st.get("filename").textValue().equals("StacktraceSerializationTest.java"))).isNotEmpty();
}

@Test
void testAppFrame() throws Exception {
when(stacktraceConfiguration.getApplicationPackages()).thenReturn(Collections.singletonList("org.example.stacktrace"));
stacktrace = getStackTrace();
Optional<JsonNode> thisMethodsFrame = stacktrace.stream()
.filter(st -> st.get("abs_path").textValue().startsWith(getClass().getName()))
.filter(st -> st.get("module").textValue().equals(getClass().getPackageName()))
.findAny();
assertThat(thisMethodsFrame).isPresent();
assertThat(thisMethodsFrame.get().get("library_frame").booleanValue()).isFalse();
Expand All @@ -92,7 +92,7 @@ void testAppFrame() throws Exception {
@Test
void testNoAppFrame() {
Optional<JsonNode> thisMethodsFrame = stacktrace.stream()
.filter(st -> st.get("abs_path").textValue().startsWith(getClass().getName()))
.filter(st -> st.get("module").textValue().startsWith(getClass().getPackageName()))
.findAny();
assertThat(thisMethodsFrame).isPresent();
assertThat(thisMethodsFrame.get().get("library_frame").booleanValue()).isTrue();
Expand All @@ -106,11 +106,46 @@ void testFileNamePresent() {
@Test
void testNoInternalStackFrames() {
assertSoftly(softly -> {
softly.assertThat(stacktrace.stream().filter(st -> st.get("abs_path").textValue().contains("java.lang.reflect."))).isEmpty();
softly.assertThat(stacktrace.stream().filter(st -> st.get("abs_path").textValue().contains("sun."))).isEmpty();
softly.assertThat(stacktrace.stream().filter(st -> st.get("module").textValue().startsWith("java.lang."))).isEmpty();
softly.assertThat(stacktrace.stream().filter(st -> st.get("module").textValue().startsWith("sun."))).isEmpty();
});
}

@Test
void testStackTraceElementSerialization() throws IOException {
when(stacktraceConfiguration.getApplicationPackages()).thenReturn(Collections.singletonList("co.elastic.apm"));

StackTraceElement stackTraceElement = new StackTraceElement("co.elastic.apm.test.TestClass",
"testMethod", "TestClass.java", 34);
String json = serializer.toJsonString(stackTraceElement);
JsonNode stackTraceElementParsed = objectMapper.readTree(json);
assertThat(stackTraceElementParsed.get("filename").textValue()).isEqualTo("TestClass.java");
assertThat(stackTraceElementParsed.get("function").textValue()).isEqualTo("testMethod");
assertThat(stackTraceElementParsed.get("library_frame").booleanValue()).isFalse();
assertThat(stackTraceElementParsed.get("lineno").intValue()).isEqualTo(34);
assertThat(stackTraceElementParsed.get("module").textValue()).isEqualTo("co.elastic.apm.test");

stackTraceElement = new StackTraceElement("co.elastic.TestClass",
"testMethod", "TestClass.java", 34);
json = serializer.toJsonString(stackTraceElement);
stackTraceElementParsed = objectMapper.readTree(json);
assertThat(stackTraceElementParsed.get("library_frame").booleanValue()).isTrue();
assertThat(stackTraceElementParsed.get("module").textValue()).isEqualTo("co.elastic");

stackTraceElement = new StackTraceElement(".TestClass",
"testMethod", "TestClass.java", 34);
json = serializer.toJsonString(stackTraceElement);
stackTraceElementParsed = objectMapper.readTree(json);
assertThat(stackTraceElementParsed.get("module").textValue()).isEqualTo("");

stackTraceElement = new StackTraceElement("TestClass",
"testMethod", "TestClass.java", 34);
json = serializer.toJsonString(stackTraceElement);
stackTraceElementParsed = objectMapper.readTree(json);
assertThat(stackTraceElementParsed.get("module").textValue()).isEqualTo("");
}


private List<JsonNode> getStackTrace() throws IOException {
final Transaction transaction = tracer.startTransaction(TraceContext.asRoot(), null, getClass().getClassLoader());
final Span span = transaction.createSpan();
Expand Down