Skip to content

Commit fd08777

Browse files
bric3devflow.devflow-routing-intake
bric3
and
devflow.devflow-routing-intake
authored
Fix incorrect os_info reporting in crash tracker (#10948)
fix: os_info values are now correctly reported according to RFC RFC: https://github.com/DataDog/libdatadog/blob/main/docs/RFCs/0013-crashtracker-errors-intake-payload.md fix: Simplify os_info data acquisition, crashlog is not necessary at all chore: empty commit style: Adds some comments Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent 796f84c commit fd08777

14 files changed

Lines changed: 62 additions & 174 deletions

File tree

‎dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/dto/CrashLog.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public final class CrashLog {
1313
public static final JsonAdapter<CrashLog> ADAPTER;
1414

1515
static {
16-
Moshi moshi = new Moshi.Builder().add(new SemanticVersion.SemanticVersionAdapter()).build();
16+
Moshi moshi = new Moshi.Builder().build();
1717
ADAPTER = moshi.adapter(CrashLog.class);
1818
}
1919

‎dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/dto/OSInfo.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ public final class OSInfo {
1111
@Json(name = "os_type")
1212
public final String osType;
1313

14-
public final SemanticVersion version;
14+
public final String version;
1515

16-
public OSInfo(String architecture, String bitness, String osType, SemanticVersion version) {
16+
public OSInfo(String architecture, String bitness, String osType, String version) {
1717
this.architecture = architecture;
1818
this.bitness = bitness;
1919
this.osType = osType;
@@ -41,10 +41,13 @@ public int hashCode() {
4141
}
4242

4343
public static OSInfo current() {
44+
String rawBitness = SystemProperties.get("sun.arch.data.model");
45+
String bitness = rawBitness != null ? rawBitness + "-bit" : null;
46+
String osName = SystemProperties.get("os.name");
47+
if (osName != null && osName.startsWith("Mac OS")) {
48+
osName = "Mac OS";
49+
}
4450
return new OSInfo(
45-
SystemProperties.get("os.arch"),
46-
SystemProperties.get("sun.arch.data.model"),
47-
SystemProperties.get("os.name"),
48-
SemanticVersion.of(SystemProperties.get("os.version")));
51+
SystemProperties.get("os.arch"), bitness, osName, SystemProperties.get("os.version"));
4952
}
5053
}

‎dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/dto/SemanticVersion.java‎

Lines changed: 0 additions & 101 deletions
This file was deleted.

‎dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/parsers/HotspotCrashLogParser.java‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ enum State {
6363
REGISTERS,
6464
SEEK_DYNAMIC_LIBRARIES,
6565
DYNAMIC_LIBRARIES,
66+
SYSTEM,
6667
DONE
6768
}
6869

@@ -75,7 +76,6 @@ public HotspotCrashLogParser() {
7576
private static final Pattern PLUS_SPLITTER = Pattern.compile("\\+");
7677
private static final Pattern SPACE_SPLITTER = Pattern.compile("\\s+");
7778
private static final Pattern NEWLINE_SPLITTER = Pattern.compile("\n");
78-
private static final Pattern SIGNAL_PARSER = Pattern.compile("\\s*(\\w+) \\((\\w+)\\).*");
7979
// Groups: 1=si_signo, 2=signal name, 3=si_code, 4=si_code name,
8080
// 5=si_addr (null for SI_USER), 6=si_pid (null for si_addr), 7=si_uid (null for si_addr)
8181
private static final Pattern SIGINFO_PARSER =
@@ -252,6 +252,7 @@ public CrashLog parse(String uuid, String crashLog) {
252252
String pid = null;
253253
List<StackFrame> frames = new ArrayList<>();
254254
String datetime = null;
255+
String datetimeRaw = null;
255256
boolean incomplete = false;
256257
String oomMessage = null;
257258
Map<String, String> registers = null;
@@ -270,9 +271,9 @@ public CrashLog parse(String uuid, String crashLog) {
270271
if (line.toLowerCase().contains("core dump")) {
271272
// break out of the message block
272273
state = State.HEADER;
273-
} else if ((oomMessage == null
274+
} else if (oomMessage == null
274275
&& (sigInfo == null || "INVALID".equals(sigInfo.name))
275-
&& !"#".equals(line))) {
276+
&& !"#".equals(line)) {
276277
// note: some jvm might use INVALID to represent a OOM crash too.
277278
final int oomIdx = line.indexOf(OOM_MARKER);
278279
if (oomIdx > 0) {
@@ -309,8 +310,10 @@ public CrashLog parse(String uuid, String crashLog) {
309310
break;
310311
case STACKTRACE:
311312
if (line.startsWith("siginfo:")) {
313+
// spotless:off
312314
// siginfo: si_signo: 11 (SIGSEGV), si_code: 1 (SEGV_MAPERR), si_addr: 0x70
313315
// siginfo: si_signo: 11 (SIGSEGV), si_code: 0 (SI_USER), si_pid: 554848, si_uid: 1000
316+
// spotless:on
314317
final Matcher siginfoMatcher = SIGINFO_PARSER.matcher(line);
315318
if (siginfoMatcher.matches()) {
316319
Integer number = safelyParseInt(siginfoMatcher.group(1));
@@ -349,13 +352,15 @@ public CrashLog parse(String uuid, String crashLog) {
349352
case SEEK_DYNAMIC_LIBRARIES:
350353
if (line.startsWith("Dynamic libraries:")) {
351354
state = State.DYNAMIC_LIBRARIES;
355+
} else if (line.contains("S Y S T E M")) {
356+
state = State.SYSTEM;
352357
} else if (line.equals("END.")) {
353358
state = State.DONE;
354359
}
355360
break;
356361
case DYNAMIC_LIBRARIES:
357362
if (line.isEmpty()) {
358-
state = State.DONE;
363+
state = State.SEEK_DYNAMIC_LIBRARIES;
359364
}
360365
final Matcher matcher = DYNAMIC_LIBS_PATH_PARSER.matcher(line);
361366
if (matcher.matches()) {
@@ -369,6 +374,16 @@ public CrashLog parse(String uuid, String crashLog) {
369374
}
370375
}
371376
break;
377+
case SYSTEM:
378+
if (line.equals("END.")) {
379+
state = State.DONE;
380+
} else if (datetime == null && datetimeRaw == null && line.startsWith("time: ")) {
381+
// JDK 8 fallback: no SUMMARY section, time is split across two lines here
382+
datetimeRaw = line.substring(6).trim();
383+
} else if (datetime == null && datetimeRaw != null && line.startsWith("timezone: ")) {
384+
datetime = dateTimeToISO(datetimeRaw + " " + line.substring(10).trim());
385+
}
386+
break;
372387
case DONE:
373388
// skip
374389
buildIdCollector.awaitCollectionDone(5);
@@ -379,7 +394,8 @@ public CrashLog parse(String uuid, String crashLog) {
379394
}
380395
}
381396

382-
if (state != State.DONE) {
397+
// SEEK_DYNAMIC_LIBRARIES and SYSTEM sections are late enough that all critical data is captured
398+
if (state != State.DONE && state != State.SEEK_DYNAMIC_LIBRARIES && state != State.SYSTEM) {
383399
// incomplete crash log
384400
incomplete = true;
385401
}

‎dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/parsers/J9JavacoreParser.java‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
*
3737
* <ul>
3838
* <li>TITLE - Contains dump event type and timestamp
39-
* <li>GPINFO - General information including OS level and CPU architecture
4039
* <li>ENVINFO - Environment info including process ID
4140
* <li>THREADS - Thread information and stack traces
4241
* </ul>
@@ -59,7 +58,6 @@ public J9JavacoreParser() {
5958
// Section markers
6059
private static final String SECTION_MARKER = "0SECTION";
6160
private static final String SECTION_TITLE = "TITLE";
62-
private static final String SECTION_GPINFO = "GPINFO";
6361
private static final String SECTION_ENVINFO = "ENVINFO";
6462
private static final String SECTION_THREADS = "THREADS";
6563

@@ -80,15 +78,13 @@ public J9JavacoreParser() {
8078
private static final Pattern NATIVE_STACK_PATTERN = Pattern.compile("4XENATIVESTACK\\s+(.+)");
8179
private static final Pattern EXCEPTION_DETAIL_PATTERN =
8280
Pattern.compile("1TISIGINFO.*[Dd]etail\\s+\"(.+?)\".*");
83-
8481
// Date time formatter for J9 format: YYYY/MM/DD at HH:MM:SS
8582
private static final DateTimeFormatter J9_DATETIME_FORMATTER =
8683
DateTimeFormatter.ofPattern("yyyy/MM/dd HH:mm:ss", Locale.ROOT);
8784

8885
enum Section {
8986
NONE,
9087
TITLE,
91-
GPINFO,
9288
ENVINFO,
9389
THREADS,
9490
OTHER
@@ -144,12 +140,6 @@ public CrashLog parse(String uuid, String javacoreContent) {
144140
}
145141
break;
146142

147-
case GPINFO:
148-
// OS level and CPU architecture are available in GPINFO section but currently
149-
// not extracted since OSInfo.current() is used for the crash report.
150-
// If needed in the future, parse 2XHOSLEVEL and 3XHCPUARCH tags here.
151-
break;
152-
153143
case ENVINFO:
154144
// Extract process ID
155145
Matcher pidMatcher = PID_PATTERN.matcher(line);
@@ -286,8 +276,6 @@ private static Integer safelyParseInt(String value) {
286276
private static Section detectSection(String line) {
287277
if (line.contains(SECTION_TITLE)) {
288278
return Section.TITLE;
289-
} else if (line.contains(SECTION_GPINFO)) {
290-
return Section.GPINFO;
291279
} else if (line.contains(SECTION_ENVINFO)) {
292280
return Section.ENVINFO;
293281
} else if (line.contains(SECTION_THREADS)) {

‎dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/CrashUploaderTest.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import static datadog.crashtracking.CrashUploader.HEADER_DD_EVP_SUBDOMAIN;
44
import static datadog.crashtracking.CrashUploader.HEADER_DD_TELEMETRY_API_VERSION;
55
import static datadog.crashtracking.CrashUploader.TELEMETRY_API_VERSION;
6-
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
76
import static org.junit.jupiter.api.Assertions.assertEquals;
87
import static org.junit.jupiter.api.Assertions.assertFalse;
98
import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -217,7 +216,7 @@ public void testErrorTrackingCrashPing() throws Exception {
217216
assertNotNull(osInfo.get("architecture"));
218217
assertNotNull(osInfo.get("version"));
219218
assertNotNull(osInfo.get("os_type"));
220-
assertDoesNotThrow(() -> Long.parseLong((String) osInfo.get("bitness"))); // 32 or 64 typically
219+
assertTrue(((String) osInfo.get("bitness")).matches("\\d+-bit")); // e.g. "64-bit"
221220

222221
// assert ddtags
223222
String ddtags = (String) extracted.remove("ddtags");
@@ -333,7 +332,7 @@ public void testErrorTrackingHappyPath(String log) throws Exception {
333332
assertNotNull(osInfo.get("architecture"));
334333
assertNotNull(osInfo.get("version"));
335334
assertNotNull(osInfo.get("os_type"));
336-
assertDoesNotThrow(() -> Long.parseLong((String) osInfo.get("bitness"))); // 32 or 64 typically
335+
assertTrue(((String) osInfo.get("bitness")).matches("\\d+-bit")); // e.g. "64-bit"
337336

338337
// assert ddtags
339338
String ddtags = (String) extracted.remove("ddtags");

‎dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/dto/SemanticVersionTest.java‎

Lines changed: 0 additions & 29 deletions
This file was deleted.

‎dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/parsers/HotspotCrashLogParserTest.java‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,15 @@
1414
import java.util.UUID;
1515
import java.util.stream.Collectors;
1616
import org.junit.jupiter.api.Test;
17-
import org.junit.jupiter.params.ParameterizedTest;
18-
import org.junit.jupiter.params.provider.CsvSource;
17+
import org.tabletest.junit.TableTest;
1918

2019
public class HotspotCrashLogParserTest {
21-
@ParameterizedTest
22-
@CsvSource({
23-
// From sample-crash-for-telemetry - 2 digits time offset
24-
"Tue Oct 17 20:25:14 2023 +08, 2023-10-17T20:25:14+08:00",
25-
// From sample-crash-for-telemetry-2 - UTC time zone name
26-
"Fri Sep 20 13:19:06 2024 UTC, 2024-09-20T13:19:06Z",
27-
// Test single digit day
28-
"Tue Oct 1 05:05:47 2024 +00:00, 2024-10-01T05:05:47Z",
29-
// Test CEST time zone name
30-
"Tue Oct 1 14:37:58 2024 CEST, 2024-10-01T14:37:58+02:00"
20+
@TableTest({
21+
"scenario | logDateTime | expectedISODateTime ",
22+
"2-digit offset | Tue Oct 17 20:25:14 2023 +08 | 2023-10-17T20:25:14+08:00",
23+
"UTC zone | Fri Sep 20 13:19:06 2024 UTC | 2024-09-20T13:19:06Z ",
24+
"single-digit day | Tue Oct 1 05:05:47 2024 +00:00 | 2024-10-01T05:05:47Z ",
25+
"CEST zone | Tue Oct 1 14:37:58 2024 CEST | 2024-10-01T14:37:58+02:00"
3126
})
3227
public void testDateTimeParser(String logDateTime, String expectedISODateTime) {
3328
assertEquals(expectedISODateTime, HotspotCrashLogParser.dateTimeToISO(logDateTime));
@@ -88,6 +83,19 @@ public void testRegisterParsingLinuxAarch64() throws Exception {
8883
assertEquals(31, crashLog.experimental.ucontext.size(), "R0-R30 = 31 registers");
8984
}
9085

86+
@TableTest({
87+
"scenario | filename ",
88+
"RHEL amd64 | sample-crash-for-telemetry.txt ",
89+
"Ubuntu amd64 | sample-crash-for-telemetry-2.txt",
90+
"macOS aarch64 | sample_oom.txt ",
91+
"macOS JDK 8 | sample-crash-for-telemetry-3.txt"
92+
})
93+
public void testOsInfoFromSystemProperties(String filename) throws Exception {
94+
final CrashLog crashLog =
95+
new HotspotCrashLogParser().parse(UUID.randomUUID().toString(), readFileAsString(filename));
96+
assertNotNull(crashLog.osInfo);
97+
}
98+
9199
private String readFileAsString(String resource) throws IOException {
92100
try (InputStream stream = getClass().getClassLoader().getResourceAsStream(resource)) {
93101
return new BufferedReader(

0 commit comments

Comments
 (0)