Skip to content

Conversation

deepfire
Copy link
Contributor

@deepfire deepfire commented Jun 5, 2020

Restore configurability that a number of traces had lost:

  • ChainDB
  • DnsSubscription
  • BlockFetchDecision
  • ForgeState
  • Forge
  • Mempool

Add configurability for the BlockchainTime trace (off by default).

deepfire added 2 commits June 5, 2020 16:24
Copy link
Contributor

@CodiePP CodiePP left a comment

Choose a reason for hiding this comment

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

the function tracerOnOff has hardcoded "MinimalVerbosity" and is not taking into account the verbosity level as set in the configuration (line 908 of Tracers.hs)
would it be possible to also pass in the TracingVerbosity to this function?

@deepfire
Copy link
Contributor Author

deepfire commented Jun 5, 2020

the function tracerOnOff has hardcoded "MinimalVerbosity" and is not taking into account the verbosity level as set in the configuration (line 908 of Tracers.hs)
would it be possible to also pass in the TracingVerbosity to this function?

Yes, that deviation of the original, obvious tracerOnOff semantic (which used to match its name) is very confusing and counterproductive.

I think that needs to be fixed, but it's probably out of scope for this PR.

cc @Jimbo4350

@deepfire
Copy link
Contributor Author

deepfire commented Jun 5, 2020

bors r+

@Jimbo4350
Copy link
Contributor

Jimbo4350 commented Jun 5, 2020

the function tracerOnOff has hardcoded "MinimalVerbosity" and is not taking into account the verbosity level as set in the configuration (line 908 of Tracers.hs)
would it be possible to also pass in the TracingVerbosity to this function?

Yes, that deviation of the original, obvious tracerOnOff semantic (which used to match its name) is very confusing and counterproductive.

I think that needs to be fixed, but it's probably out of scope for this PR.

cc @Jimbo4350

@deepfire
The current tracerOnOff mechanism will remain as is for now. The recent refactor has made things significantly clearer and we have more refactoring planned for this module. I have a branch that I will rebase on master, once this PR is merged, to fix the hardcoded trace verbosity issue.

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 5, 2020

@iohk-bors iohk-bors bot merged commit aba9ab8 into master Jun 5, 2020
@iohk-bors iohk-bors bot deleted the chasing-traces branch June 5, 2020 15:59
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.

4 participants