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
feat: Add destination property into LogEntry #720
Conversation
…me customization and integrate it with LogEntry
google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/ResourceName.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/ResourceName.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/ResourceName.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/ResourceName.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/ResourceName.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java
Outdated
Show resolved
Hide resolved
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.
Please, address a case when the log name destination is lost when parsing it from protobuf.
The code in unit test should be aligned with Java guidelines (constants naming convention and multiple use of the same string literals).
Also consider other non-blocking comments.
google-cloud-logging/src/main/java/com/google/cloud/logging/LogDestinationName.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/LogDestinationName.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java
Outdated
Show resolved
Hide resolved
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.
Good work! Couple of things to do before you merge:
- Run formatting again to make sure PR pass lint check
- Rebase to the latest version of the main branch (ping me if you have concerns about the way to do it)
Than you
Added ResourceName which will provide destination log resource name customization for LogEntry
Fixes #719 ☕️