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

Add Metrics and Target Exploration #2

Merged
merged 1 commit into from
Sep 30, 2024
Merged

Add Metrics and Target Exploration #2

merged 1 commit into from
Sep 30, 2024

Conversation

trey-ivy
Copy link
Collaborator

This is a big PR, and I do apologize. I can work on breaking it up if you prefer.

Basically I've tried to extend the datamodel in /ent/schema to be more representative of what comes in from the BES message w/regards to metrics, targets and tests. For a good visual representation of what the new schema looks like I've also add entviz which generates a graphical representation of the schema at ent/gen/ent/schema-viz.html.

Then I updated summary.go and save.go to persist the model data to the database. I'm grouping this data w/the bazel invocation object, which makes the most sense to me on where to put it.

After that I built out a few components to display the data. I used and added the recharts package for this purpose. I also built some tables that are searchable, sortable and filterable and have some color coding where it makes sense.

Obviously this still needs a lot of work, but its a start.

Runners Tab:
image

Action Cache Tab:
image

Actions Data Tab:
image

Artifacts Tab: (plans to extend this and include navigable artifacts for the build)
image

Memory Metrics Tab:
image

Timing Tab: (maybe rename to other and include network tab here)
image

Targets Tab:
image

Tests Tab:
image

System Network Metrics (optionally displayed if available)
image

@trey-ivy trey-ivy self-assigned this Sep 26, 2024
@EdSchouten
Copy link
Member

Awesome work. @trey-ivy! Please work together with @mortenmj to get this merged. The two of you now own this repo. ;-)

@mortenmj
Copy link
Collaborator

mortenmj commented Sep 27, 2024

@trey-ivy These are some really nice improvements! I tried rebasing this on #3 and had a couple of test failures. Would you mind if we merged that PR first, to get some basic CI up and running?

@trey-ivy
Copy link
Collaborator Author

no that sounds like a good plan to me

@trey-ivy trey-ivy marked this pull request as ready for review September 29, 2024 01:08
s.summary.Targets = make(map[string]TargetPair)
}

//create a target pair and at it to the targets collection
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
//create a target pair and at it to the targets collection
//create a target pair and add it to the targets collection

targetPair, ok := s.summary.Targets[label]

if !ok {
//TODO this doesn't HAVE to be fatal...timing data is just messed up and unreliable...which...its already sort of unreliable
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment is hard to understand

s.summary.Tests = make(map[string]TestsCollection)
}
testcollection, ok := s.summary.Tests[label]
if !ok { //initailize it if we've never seen this label before
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if !ok { //initailize it if we've never seen this label before
if !ok { //initialize it if we've never seen this label before

}

func readBuildGraphMetrics(buildGraphMetricsData *bes.BuildMetrics_BuildGraphMetrics) BuildGraphMetrics {
//TODO: these values are not on the proto currently. once they are, update this code to pull them out
Copy link
Collaborator

Choose a reason for hiding this comment

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

Which proto does this refer to?

@mortenmj
Copy link
Collaborator

@trey-ivy could you clean up the git history?

@trey-ivy trey-ivy merged commit 94fad13 into main Sep 30, 2024
1 check failed
@gempesaw
Copy link
Contributor

gempesaw commented Oct 3, 2024

@trey-ivy all the new statistics are really cool! thank you for contributing this PR

i have been seeing a segfault in some code that came from this PR, stacktrace below

(but .. it may also be due to my own set up? on some invocations that succeed, it says 90% of my action cache hits failed due to corrupted cache or something, so .. i'm thinking i may want to clear my CAS/AC, as i'm just using toy data anyway for now)

2024/10/03 15:28:17 INFO Stream started event="context.Background.WithValue(type transport.connectionKey, val <not Stringer>).WithValue(type peer.peerKey, val <not Stringer>).WithCancel.WithValue(type metadata.mdIncomingKey, val <not Stringer>).WithValue(type grpc.serverKey, val <not Stringer>).WithValue(type grpc.streamKey, val <not Stringer>).WithValue(type grpc.rpcInfoContextKey, val <not Stringer>)"
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x48 pc=0xe0293f]

goroutine 33 [running]:
github.com/buildbarn/bb-portal/pkg/summary.readActionCacheStatistics(...)
	/go/src/app/pkg/summary/summarizer.go:606
github.com/buildbarn/bb-portal/pkg/summary.readActionSummary(0xc0007bd880)
	/go/src/app/pkg/summary/summarizer.go:587 +0x5f
github.com/buildbarn/bb-portal/pkg/summary.Summarizer.handleBuildMetrics({0xc0001c6e08, {{0xc0003788a0, 0x2, 0x2}, {0xc0003788e0, 0x2, 0x2}}}, 0xc0008eb560)
	/go/src/app/pkg/summary/summarizer.go:416 +0x66
github.com/buildbarn/bb-portal/pkg/summary.Summarizer.ProcessEvent({0xc0001c6e08, {{0xc0003788a0, 0x2, 0x2}, {0xc0003788e0, 0x2, 0x2}}}, 0xc000378c00)
	/go/src/app/pkg/summary/summarizer.go:114 +0x54e
github.com/buildbarn/bb-portal/internal/api/grpc/bes.processBazelEvent({0x25534e8, 0xc0007bb9e0}, 0xc0007c2280, 0xc0007c2100)
	/go/src/app/internal/api/grpc/bes/bes.go:115 +0x290
github.com/buildbarn/bb-portal/internal/api/grpc/bes.BES.PublishBuildToolEventStream({0xc0001ff1e0?, {0xc0006739b0?}}, {0x255ad38, 0xc0004c5ec0})
	/go/src/app/internal/api/grpc/bes/bes.go:73 +0x245
google.golang.org/genproto/googleapis/devtools/build/v1._PublishBuildEvent_PublishBuildToolEventStream_Handler({0x1eee440?, 0xc000708b80}, {0x2558638, 0xc0007ae1e0})
	/go/pkg/mod/google.golang.org/[email protected]/googleapis/devtools/build/v1/publish_build_event.pb.go:817 +0xd8
google.golang.org/grpc.(*Server).processStreamingRPC(0xc000381400, {0x25534e8, 0xc0007bb8f0}, {0x255b900, 0xc0002be600}, 0xc00020f0e0, 0xc000673e60, 0x36483a0, 0x0)
	/go/pkg/mod/google.golang.org/[email protected]/server.go:1663 +0x1208
google.golang.org/grpc.(*Server).handleStream(0xc000381400, {0x255b900, 0xc0002be600}, 0xc00020f0e0)
	/go/pkg/mod/google.golang.org/[email protected]/server.go:1784 +0xe3a
google.golang.org/grpc.(*Server).serveStreams.func2.1()
	/go/pkg/mod/google.golang.org/[email protected]/server.go:1019 +0x8b
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 113
	/go/pkg/mod/google.golang.org/[email protected]/server.go:1030 +0x125

@trey-ivy
Copy link
Collaborator Author

trey-ivy commented Oct 7, 2024

@gempesaw hey Dan, I just saw this comment. If you could please create an issue and provide as much detail as possible I'll try to take a look. seems like we probably just need to add some null checking somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants