Skip to content
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

[BUG] - TraceMempool (and perhaps others?) config setting is ignored #3663

Closed
weebl2000 opened this issue Feb 28, 2022 · 5 comments
Closed
Labels
bug Something isn't working comp: benchmark release: 1.34.0 cardano node release 1.34.0 type: regression A feature that worked before stoped working user type: external Created by an external user

Comments

@weebl2000
Copy link

weebl2000 commented Feb 28, 2022

Internal/External
External

Area
Other cardano-node tracing config

Summary
It seems TraceMempool configuration is ignored in cardano-node 1.34.0

Steps to reproduce

  1. Upgrade to 1.34.0
  2. Set TraceMepool to false in config

Expected behavior
I do not see mempool trace logging

System info (please complete the following information):
Ubuntu 20.04 x64, cardano-node 1.34.0 built from source

Additional context
I see a lot of changes regarding tracing. Should we configure this differently? Anyway, this seems to be a regression for 1.34.0 vs 1.33.0 at least.

@weebl2000 weebl2000 added the bug Something isn't working label Feb 28, 2022
@weebl2000
Copy link
Author

https://github.com/input-output-hk/cardano-node/blob/master/configuration/cardano/mainnet-config-new-tracing.yaml relevant? Should we list this is in the changelog maybe? Seems like a heavy change if everyone needs to edit config to alter tracing.

@rdlrt
Copy link

rdlrt commented Mar 1, 2022

The release notes (or QA process - maybe it does not include testing comparison of EKG metrics against previous version given configuration for hydra builds) are a bit rushed.

The new tracing infra is summarised with a reference to a PR in changelogs, the hydra config templates referred in release notes are not updated either🙁

@weebl2000
Copy link
Author

The release notes (or QA process - maybe it does not include testing comparison of EKG metrics against previous version given configuration for hydra builds) are a bit rushed.

The new tracing infra is summarised with a reference to a PR in changelogs, the hydra config templates referred in release notes are not updated eitherslightly_frowning_face

Quite a steep learning curve to understand how to configure tracing again. :( I'll try to figure out how to disable most of the trace logging which is pretty useless for day-to-day operation.

@weebl2000
Copy link
Author

This should be fixed by #3668

iohk-bors bot added a commit that referenced this issue Mar 1, 2022
3668: Fix tracer config mismatches r=Jimbo4350 a=jbgi

Tracer mismatches, among which was mempool which has known performance impacts.

closes #3663 

Co-authored-by: Jean-Baptiste Giraudeau <jean-baptiste.giraudeau@iohk.io>
Co-authored-by: Jordan Millar <jordan.millar@iohk.io>
iohk-bors bot added a commit that referenced this issue Mar 1, 2022
3668: Fix tracer config mismatches r=Jimbo4350 a=jbgi

Tracer mismatches, among which was mempool which has known performance impacts.

closes #3663 

Co-authored-by: Jean-Baptiste Giraudeau <jean-baptiste.giraudeau@iohk.io>
Co-authored-by: Jordan Millar <jordan.millar@iohk.io>
Co-authored-by: Denis Shevchenko <denis.shevchenko@iohk.io>
@Jimbo4350
Copy link
Contributor

Resolved in #3668

@dorin100 dorin100 added type: regression A feature that worked before stoped working user type: external Created by an external user comp: benchmark release: 1.34.0 cardano node release 1.34.0 labels Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working comp: benchmark release: 1.34.0 cardano node release 1.34.0 type: regression A feature that worked before stoped working user type: external Created by an external user
Projects
None yet
Development

No branches or pull requests

4 participants