Conversation
aa1805b to
1758192
Compare
There was a problem hiding this comment.
The parser no longer strips the "Cardano" namespace prefix from config keys, which is a compatibility regression for existing configs. The previous implementation explicitly normalized "Cardano." to "" in the base version of ConfigurationParser.hs. The new code in the PR version removes that normalization entirely. Unless every caller has already stopped using Cardano.* keys, this will make previously working namespace overrides stop matching.
There was a problem hiding this comment.
Good point, and yes, this is intentional.
Internally, IOE has stopped using the Cardano.* namespace prefix for quite some time, as evidenced by https://github.com/input-output-hk/iohk-nix/blob/master/cardano-lib/generic-log-config.nix and consequently https://github.com/IntersectMBO/cardano-node/blob/master/configuration/cardano/mainnet-config.json.
External users migrating now from the legacy system won't even be tempted to use that prefix in the first place, as there's no base in the legacy tracing config to use it at all (cf. https://github.com/input-output-hk/iohk-nix/blob/master/cardano-lib/generic-log-config-legacy.nix).
Metrics is the only remaining case where this needs coverage, which is already done by the existing TraceOptionMetricsPrefix config key.
| , "TraceOptionLedgerMetricsFrequency" .= traceOptionLedgerMetricsFrequency | ||
| [ "Options" .= traceOptions | ||
| , "Forwarder" .= traceOptionForwarder | ||
| , "AppicationName" .= traceOptionNodeName |
There was a problem hiding this comment.
| , "AppicationName" .= traceOptionNodeName | |
| , "ApplicationName" .= traceOptionNodeName |
|
|
||
|
|
||
| -- In the config object, if the "HermodTracing" value is not an Object itself but a String, | ||
| -- it will be interpreted as a file path reference to the actual tracing config object. |
There was a problem hiding this comment.
HermodTracing: "path/to/file.yaml" does not work, even though the PR description and comments say it should. In [trace-dispatcher/src/Cardano/Logging/ConfigurationParser.hs], ExternalFile is only decoded from the entire document being a bare YAML string. But parseAsOuter at lines 65-67 assumes HermodTracing contains an object and never handles the string case. So a config like HermodTracing: "./tracing.yaml" is rejected instead of redirecting.
There was a problem hiding this comment.
Indeed; thank you.
Changed the instance FromJSON ExternalFile to process an object with a top-level key "HermodTracing".
Please revisit.
List of changes
PrometheusSimpleRundata type and fieldTraceConfig.tcPrometheusSimpleRun. This exposes some of the DoS protection parameters of thePrometheusSimplebackend."TracePrometheusSimpleRun": {...}. Those values will selectively override the hardcoded ones fromdefaultRunParams.Cardano.Logging.Prometheus.TCPServer.runPrometheusSimpleWithto run the backend providing a customPrometheusSimpleRunvalue.emptyTraceConfigto be actually empty."HermodTracing".!includeextension for YAML config files."TraceOptions": { "": {...}}can now be aliased as '_root_':"TraceOptions": { "_root_": {...}}. This facilitates automations that have to treat an empty JSON string as special case.Cardano.Logging.ConfigurationParser:readConfigurationWithFallbackandreadConfigurationWithFallbackAndDefault, applying fallback values to the namespace root. These will be used iff no such value was given in the file or default config, and guard against accidentally missing trace output.mkConfigurationandreadConfigurationWithFallbackconvenience functions that will yield a minimal viable configuration by applying fallback values toemptyTraceConfig.readConfiguration'andreadConfigurationWithFallback'which will silently provide a minimal viable config if the file can't be found.applyFallbackfor those who want to safeguard some hand-rolled config value.readConfigurationandreadConfigurationWithDefaultwill now guarantee fallback values:Noticeseverity,DNormaldetail level andStdout MachineFormatJSON logging.docuResultsToNamespacestoDocuGeneratorto support JSON schema generation.Furthermore, there's some general cleanup in this PR.
NB: Even though the PR introduces the new
HermodTracingconfig format, the currentcardano-nodeconfiguration cannot be represented in it, and needs to stay as-is - this would require API-breaking changes to data types. The new format targets new applications that adopt Hermod, and require a clean config interface which does not contain assumptions motivated by thecardano-nodecase.