Skip to content

New node tracing system support #594

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

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

New node tracing system support #594

wants to merge 19 commits into from

Conversation

johnalotoski
Copy link
Contributor

@johnalotoski johnalotoski commented Mar 2, 2025

This PR changes the default tracing system to the new tracing system and implements support for this where needed:

For Cardano-lib:

  • Per environment attributes of nodeConfig and nodeConfigBp now are populated with new tracing system configuration
  • New per environment attributes of nodeConfigLegacy and nodeConfigBpLegacy are available until the legacy tracing system is removed from node
  • New per environment attribute of tracerConfig is available, for use with cardano-tracer
  • New exposed top level attributes include defaultLogConfigLegacy and defaultTracerConfig
  • The mkConfigHtml function has been updated to include legacy configuration files, $ENV-config-legacy.json and $ENV-config-bp-legacy.json, and a new tracer config file, $ENV-tracer-config.json

For generic log config:

  • cardano-lib/generic-log-config.nix now holds new tracing system default config instead of legacy config

For generic tracing config:

  • cardano-lib/generic-tracer-config.nix now holds tracing system default config

For testnet template config:

  • cardano-lib/testnet-template/config.json now holds new tracing system template config instead of legacy config

For legacy generic log config:

  • cardano-lib/generic-log-config-legacy.nix now holds legacy tracing system default config

For legacy testnet template config:

  • cardano-lib/testnet-template/config-legacy.json now holds legacy tracing system template config

Other changes include:

  • The environment attribute usePeersFromLedgerAfterSlot has been renamed to useLedgerAfterSlot for consistency with the topology file key value it sets. A corresponding change has been made in the nixos cardano-node service.
  • The default auto-generated topology file name has been changed from topology.yaml to topology.json to reflect json contents.
  • Nixpkgs was bumped to a recent version and breakage changes were resolved with overlay adjustments for bomutils and nixWrapped util.

@johnalotoski johnalotoski force-pushed the jl/new-tracing branch 2 times, most recently from 8bf6d9c to 8b306d6 Compare March 19, 2025 19:57
@johnalotoski johnalotoski force-pushed the jl/new-tracing branch 2 times, most recently from 72c248d to b83a994 Compare March 31, 2025 19:56
@johnalotoski johnalotoski force-pushed the jl/new-tracing branch 2 times, most recently from b30873e to b62f65f Compare April 10, 2025 20:34
@mgmeier
Copy link

mgmeier commented Apr 23, 2025

LGTM minus the one non-existing config option.

I think it would be a nice touch to add comments everywhere, as the nix svc is meant to be a generic one. Also I hope it clarifies that settings on cardano-tracer do NOT affect the message detail or filtering of what the forwarding client (in this case, the Node) is configured to emit (in the Node config).

* Update usePeersFromLedgerAfterSlot to useLedgerAfterSlot for
  consistent naming with the cardano-node topology file key.

* It would be nice to put a deprecated warn for the legacy name in the
  environment for some time before removing it, but with eager
  evaluation of the cardanoLib environment set, either everybody sees
  the warn if present, or nobody does if missing.  Since grep.app
  doesn't find any downstream uses of the legacy name beyond the repos
  already being updated, we'll simply remove the legacy named attr now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants