feat(logs): Extend logging on generated files #2700
Conversation
kamil-bielecki-bosch
requested review from
bs-ondem,
MarcelBochtler,
mnonnenmacher,
nnobelis,
oheger-bosch,
sschuberth and
wkl3nk
as code owners
kamil-bielecki-bosch
force-pushed
the
extend-config-files-logging
branch
from
b634a8f to
37d3003
Compare
| val serviceUrl = URI.create(url).toURL() | ||
| val serviceUrl = URI.create(url).toURL() | ||
|
|
||
| val githubCredentials = buildString { |
There was a problem hiding this comment.
Nit: The variable name is misleading. This is not related to GitHub, but a plain Git configuration file. Maybe just entry.
| append(serviceUrl.protocol) | ||
| append("://") | ||
| append(builder.secretRef(usernameSecret, ConfigFileBuilder.urlEncoding)).append(':') | ||
| append(builder.secretRef(passwordSecret, ConfigFileBuilder.urlEncoding)).append('@') | ||
| append(serviceUrl.authority) | ||
| append(serviceUrl.path) | ||
| } | ||
|
|
||
| logger.debug( |
There was a problem hiding this comment.
Would it make sense to have a generic logEntry function in the builder that expects a string for the entry, the file name, and the service to make sure that all logs follow a similar structure? For instance, if it is decided to log different properties for a service, there is only a single place that needs to change.
| serviceUrl.path, | ||
| GIT_CREDENTIALS_FILE_NAME, | ||
| name, | ||
| organization, |
There was a problem hiding this comment.
Logging the full organization and product with all properties is probably too much. If this information is relevant at all, it should be sufficient to print only the name (if it is not null).
| @@ -69,6 +83,7 @@ class GitCredentialsGenerator : EnvironmentConfigGenerator<EnvironmentServiceDef | |||
| EnvironmentServiceDefinition::class.java | |||
|
|
|||
| override suspend fun generate(builder: ConfigFileBuilder, definitions: Collection<EnvironmentServiceDefinition>) { | |||
| logger.info("Generating content of {} file.", builder.getFileInUserHome(GIT_CREDENTIALS_FILE_NAME).absoluteFile) | |||
There was a problem hiding this comment.
This log statement is redundant, since the builder already logs all files that it generates. Same for the other files.
kamil-bielecki-bosch
force-pushed
the
extend-config-files-logging
branch
from
37d3003 to
3e0a2c5
Compare
kamil-bielecki-bosch
force-pushed
the
extend-config-files-logging
branch
from
3e0a2c5 to
24f4cc2
Compare
| * License-Filename: LICENSE | ||
| */ | ||
|
|
||
| package common.env |
|
|
||
| import org.slf4j.LoggerFactory | ||
|
|
||
| internal object GeneratorLogger { |
There was a problem hiding this comment.
Please add some documentation what this class is supposed to do.
kamil-bielecki-bosch
force-pushed
the
extend-config-files-logging
branch
from
24f4cc2 to
3b30072
Compare
This commit extends logging on generated configuration files: - .gitconfig - .git-credentials - .netrc - .npmrc - .yarnrc.yml - NuGet.Config - settings.xml (Maven) It helps on debug / troubleshooting when it's uncertain if file was generated, and what settings it contains. Signed-off-by: Kamil Bielecki <[email protected]>
kamil-bielecki-bosch
force-pushed
the
extend-config-files-logging
branch
from
3b30072 to
1d2cd82
Compare
This commit extends logging on generated configuration files:
It helps on debug / troubleshooting when it's uncertain if file was generated, and what settings it contains.