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.

Sign up for GitHub

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

Jump to bottom

fix: logback error show in error reporting console #43

Merged
merged 1 commit into from Apr 2, 2020
Merged

fix: logback error show in error reporting console #43

merged 1 commit into from Apr 2, 2020

Conversation

athakor
Copy link
Contributor

@athakor athakor commented Mar 3, 2020

Fixes #30

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 3, 2020
@codecov
Copy link

codecov bot commented Mar 3, 2020 •
edited

Codecov Report

Merging #43 into master will increase coverage by 1.11%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #43      +/-   ##
============================================
+ Coverage     69.17%   70.28%   +1.11%     
- Complexity       34       35       +1     
============================================
  Files             2        2              
  Lines           133      138       +5     
  Branches         14       15       +1     
============================================
+ Hits             92       97       +5     
  Misses           30       30              
  Partials         11       11
Impacted Files Coverage Δ Complexity Δ
.../google/cloud/logging/logback/LoggingAppender.java 69.69% <100%> (+1.19%) 31 <0> (+1) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 431a839...3d2c61f. Read the comment docs.

@athakor athakor requested a review from chingor13 March 3, 2020 12:27
@nguyenvietyen
Copy link

Can I help here?

I've got a vested interest in this fix too, because I want to start using Error Reporting in production.

chingor13
chingor13 approved these changes Mar 25, 2020
View reviewed changes
Copy link
Contributor

@chingor13 chingor13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable. Do we know if the string payload is backfilled by the service in case we have users who set up queries against that field?

If not, we'll mark this as behavior breaking change and release a minor version bump with a BREAKING CHANGE warning.

@chingor13 chingor13 merged commit 1baab89 into googleapis:master Apr 2, 2020
gcf-merge-on-green bot pushed a commit that referenced this pull request Apr 3, 2020
@release-please
chore: release 0.117.0 (#64)
2df607e 
🤖 I have created a release \*beep\* \*boop\* 
---
## [0.117.0](https://www.github.com/googleapis/java-logging-logback/compare/0.116.0...v0.117.0) (2020-04-02)


### Features

* expose flush() method in  LoggingAppender ([#62](https://www.github.com/googleapis/java-logging-logback/issues/62)) ([70375e1](https://www.github.com/googleapis/java-logging-logback/commit/70375e14c9c1e0c76906c87c923e23cd7f6118e8))


### Bug Fixes

* logback error show in error reporting console ([#43](https://www.github.com/googleapis/java-logging-logback/issues/43)) ([1baab89](https://www.github.com/googleapis/java-logging-logback/commit/1baab895770144febcab3384dcfb6e7bfa896787))


### Dependencies

* update core dependencies ([#11](https://www.github.com/googleapis/java-logging-logback/issues/11)) ([8c06e9a](https://www.github.com/googleapis/java-logging-logback/commit/8c06e9ab3fc44bd738e80fc966e739e8cb9f70bb))
* update core dependencies ([#18](https://www.github.com/googleapis/java-logging-logback/issues/18)) ([58f36de](https://www.github.com/googleapis/java-logging-logback/commit/58f36dec1a48f5d2a68b1327251350265941b438))
* update dependency com.google.api:api-common to v1.9.0 ([#56](https://www.github.com/googleapis/java-logging-logback/issues/56)) ([e68627a](https://www.github.com/googleapis/java-logging-logback/commit/e68627aff308f83406cd6f83fcd4c121e9d15e5c))
* update dependency com.google.cloud:google-cloud-core to v1.92.4 ([#32](https://www.github.com/googleapis/java-logging-logback/issues/32)) ([8b9574d](https://www.github.com/googleapis/java-logging-logback/commit/8b9574de2a70a5672cbfa13c3d13684066f8be05))
* update dependency com.google.cloud:google-cloud-core to v1.92.5 ([dd8b483](https://www.github.com/googleapis/java-logging-logback/commit/dd8b48348c5a04f42e963bf1cc5d427aa6e56d3a))
* update dependency com.google.cloud:google-cloud-core to v1.93.1 ([#44](https://www.github.com/googleapis/java-logging-logback/issues/44)) ([ba71ed8](https://www.github.com/googleapis/java-logging-logback/commit/ba71ed8c018b837ffd526228541f87c8ac6207f9))
* update dependency com.google.cloud:google-cloud-core to v1.93.3 ([#45](https://www.github.com/googleapis/java-logging-logback/issues/45)) ([7c30736](https://www.github.com/googleapis/java-logging-logback/commit/7c3073662bc4537c8f6a08325d52d2f1ce3dee22))
* update dependency com.google.cloud:google-cloud-logging to v1.101.0 ([#47](https://www.github.com/googleapis/java-logging-logback/issues/47)) ([585d442](https://www.github.com/googleapis/java-logging-logback/commit/585d44214e8564d8eef6f545110d244148a5f1c9))
* update dependency com.google.cloud.samples:shared-configuration to v1.0.13 ([#55](https://www.github.com/googleapis/java-logging-logback/issues/55)) ([13eb882](https://www.github.com/googleapis/java-logging-logback/commit/13eb882573a0ae91f1875b1f554809d0682e1b64))
* update dependency com.google.guava:guava-bom to v28.2-android ([#15](https://www.github.com/googleapis/java-logging-logback/issues/15)) ([2b7877d](https://www.github.com/googleapis/java-logging-logback/commit/2b7877dc4978043609a47fe0f7dfa8914c9e2622))
* update dependency org.slf4j:slf4j-api to v1.7.30 ([#12](https://www.github.com/googleapis/java-logging-logback/issues/12)) ([ce949a0](https://www.github.com/googleapis/java-logging-logback/commit/ce949a0ee0f10d1276ce3da94230c6e5f57e7b4b))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please).
@elefeint
Copy link

For posterity, textPayload does not get backfilled (one of spring-cloud-gcp integration tests stubbed a toe on this today :)

This was referenced May 19, 2020
Merged
Merged
elefeint added a commit to spring-attic/spring-cloud-gcp that referenced this pull request May 19, 2020
@elefeint
update test to retrieve json format (#2378)
74f2bbb 
Same fix as #2377, but for logging sample test.

The logging format switched from text to json in com.google.cloud.google-cloud-logging-logback:v0.117.0 (googleapis/java-logging-logback#43).

This PR fixes the trace integration test by switching from reading textPayload to jsonPayload for verification of trace correlation to log.
@meltsufin
Copy link
Member

meltsufin commented May 20, 2020 •
edited

@chingor13 This is breaking change for sure! Can we please just add a flag to LoggingAppender to switch between jsonPayload and textPayload, and leave the default as textPayload?

kateryna216 added a commit to kateryna216/spring-cloud-gcp that referenced this pull request Oct 20, 2022
@kateryna216
update test to retrieve json format (GoogleCloudPlatform#2378)
ac3c7bf 
Same fix as GoogleCloudPlatform#2377, but for logging sample test.

The logging format switched from text to json in com.google.cloud.google-cloud-logging-logback:v0.117.0 (googleapis/java-logging-logback#43).

This PR fixes the trace integration test by switching from reading textPayload to jsonPayload for verification of trace correlation to log.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chingor13 chingor13 chingor13 approved these changes

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

JavaLoggingLogback errors do not appear in StackDriver Error Reporting
6 participants
@athakor @nguyenvietyen @elefeint @meltsufin @chingor13 @googlebot