tetragon: improve protobuf usage to remove monolithic proto definitions #213
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrfastab
force-pushed
the
pr/jrfastab/protobuf
branch
from
9a7679f to
dbff22e
Compare
willfindlay
reviewed
Jun 30, 2022
kkourt
approved these changes
Jul 1, 2022
There was a problem hiding this comment.
Thanks! Just some nits from my side.
Assuming tests are ✅ and @willfindlay's comments are addressed, LGTM!
jrfastab
force-pushed
the
pr/jrfastab/protobuf
branch
from
dbff22e to
32892fb
Compare
Currently, the testing infrastructure helpers always use the canned base set of programs. This is good usually, but I'm working on a stack trace extensions for tetragon and want to port some existing networking tools into the tetragon framework. These have different sets of base programs. So to reuse the framework lets allow tests to specify their base set of programs. Signed-off-by: John Fastabend <[email protected]>
Its a bit of an abstraction leak to have the observer doing per type accounting lets push it into the sensors then they can count all sorts of interesting and useful metrics beyond just a byte was received. For now I'm just dropping these we have counters for this in ./pkg/metrics anyways. Signed-off-by: John Fastabend <[email protected]>
To avoid .type switch statements in the process and event cache this adds some extensions to the tetragon api. This will then be used to define an interface we can use to abstract some copy paste logic. Signed-off-by: John Fastabend <[email protected]>
jrfastab
force-pushed
the
pr/jrfastab/protobuf
branch
2 times, most recently
from
0a7ada8 to
da15c3d
Compare
processManager and eventCache have per type switch statements that
cut'n'paste the logic through to each type. Instead lets create
a interface for message handlers and then we can avoid caring about
event specifics in otherwise generic code.
The hope is this lets us abstract the specifics of any given event
out of the various caches and core logic we maintain. And as a
result we also can drop some open coded interface{} types with
more specific types.
This also gets us closer to allowing stacked sensors.
One point worth noting is that we the New() constructors for the
grpc/* are getting a bit strange now that the grpc object is not
used in Handlers() we might just push those into the __init func
and then we can skip the creator. I don't have any plans or
reason to support multiple constructors on the same sensor.
Signed-off-by: John Fastabend <[email protected]>
We no longer need the DoHandler logic so lets remove it. Signed-off-by: John Fastabend <[email protected]>
Use reflect pkg to do this without having to generate any specific code. Signed-off-by: John Fastabend <[email protected]>
Add makefile commands to build protoc-gen-go-tetragon. We are about to improve protoc-gen-go-tetragon and its nice to have a Makefile builder to use. Signed-off-by: John Fastabend <[email protected]>
This teaches protoc-gen-go-tetragon how to handle multiple proto files so we can avoid the monolothic .proto file definitions. Signed-off-by: John Fastabend <[email protected]>
We can run debug/CI tooling over the protobuf data to make pretty printers and such. In the process though lets break up the proto buf into logical blocks to make it easier to import. This also uses the -M option to specify the pkg to make above easier. Arguably we could generate the command from just the list of *.proto files, but I don't mind the long hand for it and I think its easy enough to read and update. If we get lots more protos we can improve it. Signed-off-by: John Fastabend <[email protected]>
Update vendor pkgs. Signed-off-by: John Fastabend <[email protected]>
The DNS pkg is only partially working at the moment and causing more confusion then good. I intended to pull it out before pushing the initial patch so it could be completed. For now revert it so we don't use up resources for something thats not implemented. For now if you need this hook to the DNS feed directly. Signed-off-by: John Fastabend <[email protected]>
jrfastab
force-pushed
the
pr/jrfastab/protobuf
branch
from
da15c3d to
c6eb1a1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Its easier to read and nicer to work on when the proto defs are not one monolithic proto with everything included in a single proto. Further, I'm looking at doing some testing/CI tooling around this so if we can just import the necessary proto files things look cleaner.
As part of the testing infra I wanted ability to test subsets of the base BPF codes. This is helpful to isolate the programs and test functionality in isolation. We add support for loading a defined set of base programs here which can easily be just a single program we want to test.
Finally, some of the codegen was not strictly necessarily so turn this into reflect.* bits to avoid doing unnecessary codegen.