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

Tracing Standards and Refactoring to Comply #235

Conversation

Modularius
Copy link
Contributor

@Modularius Modularius commented Aug 16, 2024

Summary of changes

New document on tracing, and various changes to implement it and general refactoring.

General

  1. Moved TIMESTAMP_FORMAT constant to common crate.
  2. Introduced SpannedAggregator trait to encapsulate the span-aggregating behaviour of digitiser_aggregator::Frame and nexus_writer::Run, and implemented these in digitiser_aggregator and nexus_writer.
  3. Removed FindSpan and FindSpanMut traits.
  4. Added record_metadata_fields_to_span macro to populate spans which are setup to record metadata fields. Implemented in nexus_writer, digitiser_aggregator and trace_to_events
  5. Temporarily added custom PartialEq implementation for FrameMetadata until we correct the veto_flag issue. See Restore Veto Flags in FrameMetadata comparison operations. #233
  6. Implemented the span and tracing standards outlined in tracing.md in simulator, nexus_writer, digitiser_aggregator and trace_to_events.

Docs

  1. Added tracing.md to the docs folder.

Digitiser Aggregator

  1. Refactored digitiser_aggregator::Cache struct to use HashMap rather than VecDeque
  2. Cache::push returns PartialFrame as impl SpannedAggregator.

Simulator

  1. Added IntConstant and TextConstant JSON enums that can accept either a literal value or one specified by environment variables.
  2. EventList and Trace structs added.

Instruction for review/testing

  1. Does tracing.md make sense, and is informative to a user familiar with the pipeline and with "some" knowledge of tracing/OpenTelemetry?
  2. Does the structure in the diagrams correspond to the span structure in the code? (I have tested this myself in Jaeger to ensure the end result looks right).
  3. General Code Review.

Closes #153.

@Modularius Modularius requested a review from DanNixon August 16, 2024 13:46
@DanNixon
Copy link
Member

DanNixon commented Aug 23, 2024

This is quite a big PR which could quite easily be 8 individual PRs (general 1+3, 2, 4, 5, 6, docs, aggregator, simulator), making review much easier.

Could this be split up as such?

@Modularius Modularius closed this Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Create standard for instrumentation of code
2 participants