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

Improve output when PMD is invalid #385

Open
tschmidtb51 opened this issue Jun 30, 2023 · 4 comments
Open

Improve output when PMD is invalid #385

tschmidtb51 opened this issue Jun 30, 2023 · 4 comments

Comments

@tschmidtb51
Copy link
Collaborator

Currently, we need to find a valid PMD to run the checks. However, that does not help the user, if he made a mistake in creating the PMD. We should provide more insights (JSON parse, JSON schema validation, etc.) what went wrong to support the users.

@tschmidtb51 tschmidtb51 added enhancement New feature or request csaf_checker labels Jun 30, 2023
@tschmidtb51
Copy link
Collaborator Author

Out of scope for 2.2.0

@oxisto
Copy link
Contributor

oxisto commented Apr 23, 2024

We were discussing about this internally... Are you referring to the details that are gathered internally, for example here:
https://github.com/csaf-poc/csaf_distribution/blob/d909e9de151d5845fe0c0d5b9db2152f9db25e90/cmd/csaf_checker/processor.go#L501-L509

or here

https://github.com/csaf-poc/csaf_distribution/blob/d909e9de151d5845fe0c0d5b9db2152f9db25e90/cmd/csaf_checker/processor.go#L525-L527

The latter already includes all error message related to JSON parsing and validation and publishes it to the "badProviderMetadata" feed.

Currently, it seems that this is already gathered in the topics, but there is no way to actually output the error message. Instead, only Could not parse the Provider-Metadata.json of is printed. Would it make sense to tie-in the structured logging into the "topics" and then provide error/warn/info log output based on the contents of the topic. If you are interested, my colleague @lebogg can have a look at this.

@lebogg
Copy link

lebogg commented Apr 24, 2024

As @oxisto already mentioned, a possible and trivial solution would be to add logging to the topicMessages' methods:

@@ -127,16 +127,22 @@ func (m *topicMessages) add(typ MessageType, format string, args ...any) {
 // error adds an error message to this topic.
 func (m *topicMessages) error(format string, args ...any) {
        m.add(ErrorType, format, args...)
+       // Print messages to notify user what went wrong
+       slog.Error(format, args)
 }
 
 // warn adds a warning message to this topic.
 func (m *topicMessages) warn(format string, args ...any) {
        m.add(WarnType, format, args...)
+       // Log message to warn user what maybe went wrong
+       slog.Warn(format, args)
 }
 
 // info adds an info message to this topic.
 func (m *topicMessages) info(format string, args ...any) {
        m.add(InfoType, format, args...)
+       // Log message to notify user what happened
+       slog.Info(format, args)
 }
 
 // use signals that we going to use this topic.

Or did you have something else in mind?

@bernhardreiter bernhardreiter added service+dev and removed enhancement New feature or request labels Sep 16, 2024
@bernhardreiter
Copy link
Member

Sounds good to allow the details of failed JSON parsing or schema check to be communicated to the user. This is for the PMD, but we should also do it for the documents itself, see sister issue #572.

We should check if the downloader and aggregator should also get that diagnostic messages (so they can enable it) or if they could get a message like: Use the checker to see more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants