Skip to content

Conversation

@sanderpick
Copy link
Collaborator

  • Allow passing domain_filter and events_filter via env
  • Allow passing lowercase log level, e.g., "info", instead of "Info"

Signed-off-by: Sander Pick <sanderpick@gmail.com>
@sanderpick sanderpick requested a review from a team as a code owner October 2, 2024 18:11
#[serde_as]
#[derive(Debug, Deserialize, Clone, Default, strum::EnumString, strum::Display)]
#[strum(serialize_all = "snake_case")]
#[serde(rename_all = "lowercase")]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

apparently #[strum(serialize_all = "snake_case")] only applies to and from string, not with serde.

Comment on lines +337 to +338
.with_list_parse_key("eth.tracing.file.domain_filter")
.with_list_parse_key("eth.tracing.file.events_filter"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these necessary for the Eth API process? I think it should just reuse the general tracing configuration, and if we want to enable/disable Eth API tracing, we should have an Eth domain we can disable through the domain_filter.

Copy link
Collaborator Author

@sanderpick sanderpick Oct 2, 2024

Choose a reason for hiding this comment

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

Nice, that sounds better. So maybe EthSettings should just have a subset of the tracing settings? as in:

pub domain_filter: Option<Vec<String>>,
pub events_filter: Option<Vec<String>>,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm happy to make that change if sounds right... would prefer to do in a diff PR.

Signed-off-by: Sander Pick <sanderpick@gmail.com>
max_log_files = 5 # Number of files to keep after rotation
rotation = "daily" # Options: minutely, hourly, daily, never
## Optional: filter events by domain
domain_filter = "Bottomup, Consenesus, Mpool, Execution, Topdown, TracingError"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

looking for instances of impl_traceables!, I didn't find one with "TracingError", but there was one with "System"

Co-authored-by: raulk <raul.kripalani@gmail.com>
@karlem karlem merged commit 4b2eb9e into consensus-shipyard:main Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants