Change Log Severity from Debug for pipeline or step creation errors

[flimmy]

Hey Everyone,

i would like to propose a change to the log level of several "debug" messages, as the debug log level is not suitable for a production setup and some IMHO important information gets lost.
As the perceived importance is inherently subjective any input is welcome.
Here are my 2 cents:

message	prev. Lvl	new Lvl	Reasoning
ignoring pipeline as skip-ci was found ...	debug	info	skips are not visible in the browser, so make them visible in log
cannot find config '%s' in '%s' with ...	debug	info	no config maybe a misconfigured lookup, so make it visible to the sysadmin
failed to parse yaml	debug	warn	a failure should be at least a warning
ErrFiltered (ignoring hook: 'when' filters filtered out all steps)	debug	info	the most common request i get as a sysadmin "why does it not start"
marked as skipped, does not match metadata	debug	info	same as above, but more context
pipeline config could not be parsed	debug	warn	a failure should be at least a warning

Edit 1: strike the 2 proposed warning changes as these are already shown in the web-ui

[qwerty287]

Thanks. Though, I'm quite sure we changed this to prevent spamming the logs on large/public instances with many repos.

[flimmy]

i figured, but didn't find a issue or pr for these, and the debug log is too huge too leave enabled.
Would "info" for these be a possible compromise, since i would expect most public instances to have the default log level set to warn or higher.
Else feel free to close the pr, or leave it for some more input from others.
Regards, Flimmy

[xoxys]

I would agree at least to some changes. Some of the logs e.g. failed to parse yaml should also be covered by the linter and show up in the UI for users already, or am I wrong? At least for such cases I dont see a need to increase the log level.

[lafriks]

I also agree that most of these should be UI errors

[6543]

I also agree that instead of spaming the logs we should have it propergated to the UI in a nice way

[flimmy]

right, the two entries with a "failure" do exactly that, they create a pipeline entry with an error tab i'll strike them in the main post and the change.

The main problem for me are the ones that do not produce any output at all (unless you have debug logs enabled).

Like this one: "ErrFiltered (ignoring hook: 'when' filters filtered out all steps)"

These are the ones that produce the most work for me as an admin.
So would changing the ones that "silently" fail/drop from debug to info be an option (i'll go and identify them and then change the pr accordingly)?

Alternatively it might be cool to create "empty" piplines entries with an error tab, just like the yaml parsing failures do. If that is something you would prefer, i would create a new pr/issue (or change this one instead of closing to preserve this conversation), as for that i would need more time because i'm not yet that familiar with the codebase.

[flimmy]

rebased to upstream and reverted the loglevel change for the two errors that already generate a message in the webui.

Further research on the remaining 4 changes:

message	line	in-code reason for silent drop
ignoring pipeline as skip-ci was found ...	51	return nil, ErrFiltered instead of a pipeline with error like in line 97
cannot find config '%s' in '%s' with ...	83	return nil, ErrFiltered instead of a pipeline with error like in line 97
ErrFiltered (ignoring hook: 'when' filters filtered out all steps)	103	return nil, ErrFiltered instead of a pipeline with error like in line 97
marked as skipped, does not match metadata	165	return nil, nil instead of appending an error like in line 173

If returning the pipeline and an error for the messages in server/pipeline/create.go or generating and returning an err in server/pipeline/stepbuilder/step_builder.go is a "quick fix" for one of you, or something you would see yourself working on in the near future, please let me know, else i will try to find some time to have a deeper look into the codebase to undestand the implications of creating incomplete pipeline objects.

Or maybe we can merge the current loglevel changes and i create a seperate issue to "add ui warnings for silently dropped pipelines".

[lafriks]

I would be against this as this would result in a large amount of unneeded logs for large instances, if needed this should be added maybe as an option that by default is off

[flimmy]

I would be against this as this would result in a large amount of unneeded logs for large instances, if needed this should be added maybe as an option that by default is off

what exactly?: the increased log level, or the addition of a ui warning and thus a database entry

If its the first: i would like to argue that log-level "info" is not suitable for large installations anyway (in my experience error or warn are being used), and since there is nothing between info and debug (which is not suitable for any production setup no matter the size), i personally feel like "info" is a good compromise to silently dropping it and thus generating work for people trying to figure out why their pipeline wont start.

[lafriks]

Just an idea, don't know if the best option but could be additional server option LOG_SKIPPED_WORKFLOWS that if set would log these into (info/warn) level otherwise continue to log these as debug.

[xoxys]

At least this part should be configurable. I dont want a failed pipeline or spammed log in case a pipeline was skipped by when filters as for me thats expected behaviour.

[flimmy]

At least this part should be configurable. I dont want a failed pipeline or spammed log in case a pipeline was skipped by when filters as for me thats expected behaviour.

like this?

I added the target loglevel as an env-var in the flags and added it to the pipeline config struct in server/config.go