Skip to content

Conversation

@mgmeier
Copy link
Contributor

@mgmeier mgmeier commented Nov 24, 2025

Description

This PR adds several quality-of-life features to the new tracing system, as well as improve robustness or code quality in several places. Broken down by component, the changes contained are:

cardano-tracer:

  • Implement Prometheus HTTP service discovery (SD) under the URL /targets
  • Add optional config field "prometheusLabels": { "<labelname>": "<labelvalue>", ... } for custom labels to be attached with Prometheus SD
  • Use TracerTrace.forMachine directly instead of going via derived TracerTrace.toJSON; remove unused TracerTrace JSON instances
  • Use proper 'camelCase' for machine-readable TracerTrace
  • Proper tracing (vs. dumping to stdout) for showProblemIfAny and for forwarding connection interruptions
  • Remove redundant runInLoop in favour of trace-dispatcher's implementation
  • Split up journal handler implementation into internal modules Systemd and NoSystemd (maintenance)

trace-forward:

  • Provide InitForwardingConfig config record type for initForwarding and initForwardingDelayed

trace-dispatcher:

  • class LogFormatting: remove redundant forHumanFromMachine and forHumanOrMachine (the system already does that inherently)
  • Introduce type Cardano.Logging.Types.TraceMessage.TraceMessage with explicit codecs for JSON and CBOR
  • Rework PreFormatted type and formatters to use TraceMessage; slightly optimize humanFormatter'
  • Add CBOR formatting via FormattedMessage.FormattedCBOR constructor and a cborFormatter' function
  • Replaced both disconnectedQueueSize and connectedQueueSize with queueSize in TraceOptionForwarder while keeping config parsing backwards compatible
  • Add retry delay reset in runInLoop when the action runs sufficiently long
  • Safely stop standardTracer's stdout thread when there are no more producers (re-enabling the cabal bench)

Additionally, the PR:

  • removes any use of forHumanFromMachine and forHumanOrMachine from the cardano-node codebase, replacing it with forHuman only where appropriate
  • implements proper tracing (vs. dumping to stdout) for forwarding connection interruptions in cardano-node and tx-generator

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. These may include:
    • golden tests
    • property tests
    • roundtrip tests
    • integration tests
      See Runnings tests for more details
  • Any changes are noted in the CHANGELOG.md for affected package
  • The version bounds in .cabal files are updated
  • CI passes. See note on CI. The following CI checks are required:
    • Code is linted with hlint. See .github/workflows/check-hlint.yml to get the hlint version
    • Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml to get the stylish-haskell version
    • Code builds on Linux, MacOS and Windows for ghc-9.6 and ghc-9.12
  • Self-reviewed the diff

@mgmeier mgmeier force-pushed the mkarg/new-tracing-changes-maintenance branch 2 times, most recently from f4f9ee7 to c4f7e74 Compare November 26, 2025 10:18
@mgmeier mgmeier force-pushed the mkarg/new-tracing-changes-maintenance branch from c4f7e74 to dfa664a Compare November 26, 2025 10:25
@mgmeier mgmeier marked this pull request as ready for review November 26, 2025 10:57
@mgmeier mgmeier requested review from a team as code owners November 26, 2025 10:57
Copy link
Contributor

@jutaro jutaro left a comment

Choose a reason for hiding this comment

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

A lot of changes, and I just have one question wit it. Otherwise it looks good

Copy link
Contributor

@jutaro jutaro left a comment

Choose a reason for hiding this comment

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

LGTM

@mgmeier mgmeier added this pull request to the merge queue Nov 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 28, 2025
@mgmeier mgmeier added this pull request to the merge queue Nov 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 28, 2025
@mgmeier mgmeier added this pull request to the merge queue Nov 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 28, 2025
@mgmeier mgmeier added this pull request to the merge queue Nov 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 28, 2025
@mgmeier mgmeier added this pull request to the merge queue Nov 28, 2025
Merged via the queue into master with commit deeeeea Nov 28, 2025
25 checks passed
@mgmeier mgmeier deleted the mkarg/new-tracing-changes-maintenance branch November 28, 2025 21:12
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.

3 participants