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

Introduce protobuf definitions for model types #36

Closed
4 tasks done
axw opened this issue May 12, 2023 · 2 comments
Closed
4 tasks done

Introduce protobuf definitions for model types #36

axw opened this issue May 12, 2023 · 2 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@axw
Copy link
Member

axw commented May 12, 2023

For several reasons we would like to define an efficient, stable, binary encoding for model types. e.g. we would use this for storing events in Badger for tail-based sampling. These would be much faster to encode/decode, and more importantly will have strong stability guarantees

To achieve the above, we will define our intermediate, in-memory/on-disk, model types in protobuf -- this will be the source of truth. We'll take a phased approach to this, given that the existing types are used all across apm-server, and making a big-bang change would carry a significant amount of risk of introducing bugs.

With #35 merged, we have created a cleaner separation between the model types and the way they are encoded to JSON. The model types no longer have to directly map to the final document structure, though for our sanity we should probably keep them close. The model types do not need to be ECS-compliant, and we can instead evolve the JSON encoding over time without changing the model types.

Phase 1 (iteration-05)

  • introduce protobuf definitions (probably worth basing off @marclop's work in https://github.com/elastic/apm-ingest-queueless)
  • generate Go types from protobuf (into model/modelpb or something like that)
  • introduce code for JSON encoding protobuf types by transforming to internal/modeljson types, like we're doing with model.APMEvent now
  • introduce code for mapping model events to the protobuf-generated types; remove the code for translating from model types to modeljson, and instead translate model types to protoc-generated types, then protoc-generated types to modeljson

Phase 2 (iteration-06)
#52

@simitt
Copy link
Contributor

simitt commented Jun 8, 2023

moved phase 2 to a dedicated issue, @kruskall please close this when everything from phase-1 is done

@kruskall kruskall closed this as completed Jun 8, 2023
@kruskall
Copy link
Member

kruskall commented Jun 8, 2023

The last PR was merged today so this can be closed 🎉

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

No branches or pull requests

3 participants