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

[Core] Avoid NPEs on creating events #179

Merged
merged 2 commits into from
May 23, 2024
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 @@ -117,14 +117,14 @@ public boolean addEvent(JsonObject event) {
switch (event.get(TraceEventFormatConstants.EVENT_PHASE).getAsString()) {
case TraceEventFormatConstants.PHASE_COMPLETE: // Complete events
{
completeEvents.add(new CompleteEvent(event));
completeEvents.add(CompleteEvent.fromJson(event));
break;
}

case "I": // Deprecated, fall-through
case TraceEventFormatConstants.PHASE_INSTANT: // Instant events
{
InstantEvent instantEvent = new InstantEvent(event);
InstantEvent instantEvent = InstantEvent.fromJson(event);

List<InstantEvent> instantList =
instants.compute(
Expand All @@ -142,7 +142,7 @@ public boolean addEvent(JsonObject event) {

case TraceEventFormatConstants.PHASE_COUNTER: // Counter events
{
CounterEvent counterEvent = new CounterEvent(event);
CounterEvent counterEvent = CounterEvent.fromJson(event);

List<CounterEvent> countList =
counts.compute(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,91 @@

import com.engflow.bazel.invocation.analyzer.time.TimeUtil;
import com.engflow.bazel.invocation.analyzer.time.Timestamp;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import com.google.gson.JsonObject;
import java.time.Duration;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import javax.annotation.Nullable;

/**
* Complete events describe an event with a duration.
*
* @see <a
* https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.lpfof2aylapb">specification</a>
*/
public class CompleteEvent {
@VisibleForTesting
static final List<String> REQUIRED_JSON_MEMBERS =
List.of(
TraceEventFormatConstants.EVENT_TIMESTAMP,
TraceEventFormatConstants.EVENT_DURATION,
TraceEventFormatConstants.EVENT_THREAD_ID,
TraceEventFormatConstants.EVENT_PROCESS_ID);

@Nullable public final String name;
@Nullable public final String category;
public final Timestamp start;
public final Duration duration;
public final Timestamp end;
public final int threadId;
public final int processId;
public final ImmutableMap<String, String> args;
public final Map<String, String> args;

public static CompleteEvent fromJson(JsonObject object) {
Preconditions.checkNotNull(object);
List<String> missingMembers = Lists.newArrayList();
for (String requiredMember : REQUIRED_JSON_MEMBERS) {
if (!object.has(requiredMember)) {
missingMembers.add(requiredMember);
}
}
if (!missingMembers.isEmpty()) {
throw new IllegalArgumentException(
"Missing members: " + Arrays.toString(missingMembers.toArray()));
}

return new CompleteEvent(
object.has(TraceEventFormatConstants.EVENT_NAME)
? object.get(TraceEventFormatConstants.EVENT_NAME).getAsString()
: null,
object.has(TraceEventFormatConstants.EVENT_CATEGORY)
? object.get(TraceEventFormatConstants.EVENT_CATEGORY).getAsString()
: null,
Timestamp.ofMicros(object.get(TraceEventFormatConstants.EVENT_TIMESTAMP).getAsLong()),
TimeUtil.getDurationForMicros(
object.get(TraceEventFormatConstants.EVENT_DURATION).getAsLong()),
object.get(TraceEventFormatConstants.EVENT_THREAD_ID).getAsInt(),
object.get(TraceEventFormatConstants.EVENT_PROCESS_ID).getAsInt(),
object.has(TraceEventFormatConstants.EVENT_ARGUMENTS)
? object
.get(TraceEventFormatConstants.EVENT_ARGUMENTS)
.getAsJsonObject()
.entrySet()
.stream()
.collect(toImmutableMap(e -> e.getKey(), e -> e.getValue().getAsString()))
: ImmutableMap.of());
}

/**
* Parses a {@link CompleteEvent} from a JsonObject.
*
* @deprecated Use {@link #fromJson(JsonObject)} instead.
*/
@Deprecated
public CompleteEvent(JsonObject object) {
this(
object.has(TraceEventFormatConstants.EVENT_NAME)
? object.get(TraceEventFormatConstants.EVENT_NAME).getAsString().intern()
? object.get(TraceEventFormatConstants.EVENT_NAME).getAsString()
: null,
object.has(TraceEventFormatConstants.EVENT_CATEGORY)
? object.get(TraceEventFormatConstants.EVENT_CATEGORY).getAsString().intern()
? object.get(TraceEventFormatConstants.EVENT_CATEGORY).getAsString()
: null,
Timestamp.ofMicros(object.get(TraceEventFormatConstants.EVENT_TIMESTAMP).getAsLong()),
TimeUtil.getDurationForMicros(
Expand All @@ -53,9 +115,7 @@ public CompleteEvent(JsonObject object) {
.getAsJsonObject()
.entrySet()
.stream()
.collect(
toImmutableMap(
e -> e.getKey().intern(), e -> e.getValue().getAsString().intern()))
.collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue().getAsString()))
: ImmutableMap.of());
}

Expand All @@ -66,15 +126,18 @@ public CompleteEvent(
Duration duration,
int threadId,
int processId,
ImmutableMap<String, String> args) {
this.name = name;
this.category = category;
Map<String, String> args) {
this.name = name == null ? null : name.intern();
this.category = category == null ? null : category.intern();
this.start = start;
this.duration = duration;
this.end = start.plus(duration);
this.threadId = threadId;
this.processId = processId;
this.args = args;
this.args =
args.entrySet().stream()
.collect(
Collectors.toUnmodifiableMap(e -> e.getKey().intern(), e -> e.getValue().intern()));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,78 @@
package com.engflow.bazel.invocation.analyzer.traceeventformat;

import com.engflow.bazel.invocation.analyzer.time.Timestamp;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import com.google.gson.JsonObject;
import java.util.Arrays;
import java.util.List;

/**
* Counter events can track a value or multiple values as they change over time.
*
* @see <a
* href="https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.msg3086636uq">specification</a>
*/
public class CounterEvent {
@VisibleForTesting
static final List<String> REQUIRED_JSON_MEMBERS =
List.of(
TraceEventFormatConstants.EVENT_NAME,
TraceEventFormatConstants.EVENT_TIMESTAMP,
TraceEventFormatConstants.EVENT_ARGUMENTS);

private final String name;
private final Timestamp timestamp;
private final double totalValue;

public CounterEvent(JsonObject event) {
this.name = event.get(TraceEventFormatConstants.EVENT_NAME).getAsString().intern();
this.timestamp =
Timestamp.ofMicros(event.get(TraceEventFormatConstants.EVENT_TIMESTAMP).getAsLong());
public static CounterEvent fromJson(JsonObject event) {
List<String> missingMembers = Lists.newArrayList();
for (String requiredMember : REQUIRED_JSON_MEMBERS) {
if (!event.has(requiredMember)) {
missingMembers.add(requiredMember);
}
}
if (!missingMembers.isEmpty()) {
throw new IllegalArgumentException(
"Missing members: " + Arrays.toString(missingMembers.toArray()));
}
Comment on lines +45 to +54

Choose a reason for hiding this comment

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

In a follow-up, this could become a generic helper method.


// For now we are treating all the different counts as a single metric by summing them
// together. In the future we may want to respect each count individually.
this.totalValue =
var totalValue =
event.get(TraceEventFormatConstants.EVENT_ARGUMENTS).getAsJsonObject().entrySet().stream()
.mapToDouble(e -> e.getValue().getAsDouble())
.sum();

return new CounterEvent(
event.get(TraceEventFormatConstants.EVENT_NAME).getAsString(),
Timestamp.ofMicros(event.get(TraceEventFormatConstants.EVENT_TIMESTAMP).getAsLong()),
totalValue);
}

/**
* Parses a {@link CounterEvent} from a JsonObject.
*
* @deprecated Use {@link #fromJson(JsonObject)} instead.
*/
@Deprecated
public CounterEvent(JsonObject event) {
this(
event.get(TraceEventFormatConstants.EVENT_NAME).getAsString(),
Timestamp.ofMicros(event.get(TraceEventFormatConstants.EVENT_TIMESTAMP).getAsLong()),
// For now we are treating all the different counts as a single metric by summing them
// together. In the future we may want to respect each count individually.
event.get(TraceEventFormatConstants.EVENT_ARGUMENTS).getAsJsonObject().entrySet().stream()
.mapToDouble(e -> e.getValue().getAsDouble())
.sum());
}

private CounterEvent(String name, Timestamp timestamp, double totalValue) {
this.name = Preconditions.checkNotNull(name).intern();
this.timestamp = Preconditions.checkNotNull(timestamp);
this.totalValue = totalValue;
}

public String getName() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,67 @@
package com.engflow.bazel.invocation.analyzer.traceeventformat;

import com.engflow.bazel.invocation.analyzer.time.Timestamp;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import com.google.gson.JsonObject;
import java.util.Arrays;
import java.util.List;

/**
* Instant events describe an event that has no duration.
*
* @see <a
* https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.lenwiilchoxp">specification</a>
*/
public class InstantEvent {
@VisibleForTesting
static final List<String> REQUIRED_JSON_MEMBERS =
List.of(
TraceEventFormatConstants.EVENT_CATEGORY,
TraceEventFormatConstants.EVENT_NAME,
TraceEventFormatConstants.EVENT_TIMESTAMP);

private final String category;
private final String name;
private final Timestamp timestamp;

public static InstantEvent fromJson(JsonObject event) {
List<String> missingMembers = Lists.newArrayList();
for (String requiredMember : REQUIRED_JSON_MEMBERS) {
if (!event.has(requiredMember)) {
missingMembers.add(requiredMember);
}
}
if (!missingMembers.isEmpty()) {
throw new IllegalArgumentException(
"Missing members: " + Arrays.toString(missingMembers.toArray()));
}

return new InstantEvent(
event.get(TraceEventFormatConstants.EVENT_CATEGORY).getAsString(),
event.get(TraceEventFormatConstants.EVENT_NAME).getAsString(),
Timestamp.ofMicros(event.get(TraceEventFormatConstants.EVENT_TIMESTAMP).getAsLong()));
}

/**
* Parses a {@link InstantEvent} from a JsonObject.
*
* @deprecated Use {@link #fromJson(JsonObject)} instead.
*/
@Deprecated
public InstantEvent(JsonObject event) {
this.category = event.get(TraceEventFormatConstants.EVENT_CATEGORY).getAsString().intern();
this.name = event.get(TraceEventFormatConstants.EVENT_NAME).getAsString().intern();
this.timestamp =
Timestamp.ofMicros(event.get(TraceEventFormatConstants.EVENT_TIMESTAMP).getAsLong());
this(
event.get(TraceEventFormatConstants.EVENT_CATEGORY).getAsString(),
event.get(TraceEventFormatConstants.EVENT_NAME).getAsString(),
Timestamp.ofMicros(event.get(TraceEventFormatConstants.EVENT_TIMESTAMP).getAsLong()));
}

private InstantEvent(String category, String name, Timestamp timestamp) {
this.category = Preconditions.checkNotNull(category).intern();
this.name = Preconditions.checkNotNull(name).intern();
this.timestamp = Preconditions.checkNotNull(timestamp);
}

public String getCategory() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
java_test(
name = "traceeventformat",
srcs = glob(["**/*.java"]),
test_class = "com.engflow.bazel.invocation.analyzer.traceeventformat.TraceEventFormatTestSuite",
deps = [
"//analyzer/java/com/engflow/bazel/invocation/analyzer/time",
"//analyzer/java/com/engflow/bazel/invocation/analyzer/traceeventformat",
"//third_party/gson",
"//third_party/junit",
"//third_party/truth",
],
)
Loading