-
Notifications
You must be signed in to change notification settings - Fork 513
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
Noisy logs #3202
Comments
…nt by logging statements
…nt by logging statements Signed-off-by: Ricky Ng-Adam <[email protected]>
…nt by logging statements Signed-off-by: Ricky Ng-Adam <[email protected]>
…nt by logging statements Signed-off-by: Ricky Ng-Adam <[email protected]>
The deprecation banner and notices should only be displayed when you are using deprecated features. Suppressing these messages defeats the purpose of the messages - to let you know you are using deprecated features and you should migrate to alternate (replacement) features. So, the best practice way to get rid of the messages is to migrate away from the deprecated features you're using. I do agree the messages should be logged rather than being printed. Regarding the standard ACA-Py startup banner. I think this is a very important piece of diagnostic information that should not be suppressed. It provides you with a summary of the agent configuration and version that can help you identify issues right off the bat. |
I expected that too, but if you look at the code the banner is printed everytime (there's no conditional): from here: the only condition for printing the deprecation notice is having transports configured (aka transport is not disabled) It's giving a deprecation notice whatever config and even though #2368 is not done yet. Correct me if I'm wrong, but there seems to be no way to "correctly" configure the agent to get rid of the deprecation notice. |
Perhaps in regular usage but it interferes with testing (I'm forking 100 agents from the same parent process, creating a storm of output) and creates the habit of eventually ignoring output that could/should be important. |
…nt by logging statements Signed-off-by: Ricky Ng-Adam <[email protected]>
I don't think we'd want to add another configuration for this. We're really trying not to bloat the configurations. But, printing the banner nicely with the logger will be a bit tricky because it's configured to print the timestamp and class. This seems like a pretty niche use case. The depreciation notice is only like 10 lines of output. If other maintainers don't care about adding a configuration than the open PR looks acceptable. |
I don’t like the idea of the flag to block the banner. The whole point of the messages is that they be ugly and in the face of the deployers so that they become aware of the need to evolve their deployment. Especially if they go away when the deployment removes the deprecated feature. That’s the behaviour we want to encourage — “This message will not be printed if you update your deployment to do X, Y and Z. Do it today!!”. If a deployment simply adds that flag, they will not see the future messages and will not know about future deprecations. I think we should not add this. |
@swcurran @jamshale as it is now, there's no way around the "deprecation notice". It is deprecating something without any working alternatives to upgrade to. For me, that is not deprecation, it is noise for which I cannot do anything about, polluting my stdout and causing me to miss actual important error messages. |
Why don't you just test with your own fork or image? Or you should be able to filter your log output for single instances. You could also spread your instances startups out do the logs aren't all mixed up. There is a lot of ways around this and finding these errors easier isn't something we need to fix by adding another configuration parameter. |
What deprecation are you talking about? Do you mean this one?
If so, that is not about “did:sov” vs. “did:indy”. Rather — it is the prefix to a DIDComm message type, such as: I fully agree that there should be a way to suppress such messages, and I’m not sure about this one, since the message is saying “hey, if you get a message from another party using the old prefix you should yell at them”. There is a flag to ensure that your deployment is not sending out such a prefix (since 0.5.5), and that flag is now automatically active for all ACA-Py deployments (since 0.11.0). How about we do this:
Thoughts? |
…nt by logging statements Signed-off-by: Ricky Ng-Adam <[email protected]>
…nt by logging statements Signed-off-by: Ricky Ng-Adam <[email protected]>
…nt by logging statements Signed-off-by: Ricky Ng-Adam <[email protected]>
…nt by logging statements Signed-off-by: Ricky Ng-Adam <[email protected]>
…nt by logging statements Signed-off-by: Ricky Ng-Adam <[email protected]>
…nt by logging statements Signed-off-by: Ricky Ng-Adam <[email protected]>
At the agent start, the stdout of the agent shows multiple banner messages and output from print statements.
Having noisy logs makes it more difficult to identify quickly actual problems.
Expect to:
list of print statements
https://gist.github.com/rngadam/3cccd0f515091544f745d681d8119ae4
The text was updated successfully, but these errors were encountered: