-
Notifications
You must be signed in to change notification settings - Fork 29
feat: Allow customizing written log entries by exposing parameters via appender configuration #625
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
Conversation
…logging-logback into losalex/fix-551
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a couple of discrepancies that worth addressing before the merge.
between two options to configure project id: using LoggingOptions
or providing the project id with WriteOption
, please consider selecting one.
src/main/java/com/google/cloud/logging/logback/LoggingAppender.java
Outdated
Show resolved
Hide resolved
loggingOptions = LoggingOptions.getDefaultInstance(); | ||
} else { | ||
LoggingOptions.Builder builder = LoggingOptions.newBuilder(); | ||
builder.setProjectId(projectId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since the project id is already set via WriteOption
there is no need to set it also using LoggingOptions
.
I suggest to choice either method but not both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The projectId
provided through setProjectId()
overrides the one from credentials. If projectId
was not set through setProjectId()
, the one from credentials will be used
…logging-logback into losalex/fix-551
…logging-logback into losalex/fix-551
…logging-logback into losalex/fix-551
…logging-logback into losalex/fix-551
…logging-logback into losalex/fix-551
…logging-logback into losalex/fix-551
…logging-logback into losalex/fix-551
…logging-logback into losalex/fix-551
…logging-logback into losalex/fix-551
…logging-logback into losalex/fix-551
…logging-logback into losalex/fix-551
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm.
<!-- Optional: defaults to the project id obtained during authentication process. Project id is also used to construct resource name of the log entries --> | ||
<logDestinationProjectId>String</logDestinationProjectId> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this change is done by you, please undo it to avoid unnecessary PRs since the change to .readme-partials.yaml
does the job.
If this change is submitted by Owlbot, please ignore this comment.
This feature enables customizing logs destination by setting project/folder/organization/billing destinations (see more details under logName field).
Fixes #551 ☕️