Skip to content
This repository was archived by the owner on Feb 7, 2026. It is now read-only.

fix: do not promote record.labels if improper#266

Merged
ofrobots merged 3 commits intomasterfrom
fix-265
Mar 1, 2019
Merged

fix: do not promote record.labels if improper#266
ofrobots merged 3 commits intomasterfrom
fix-265

Conversation

@ofrobots
Copy link
Copy Markdown
Contributor

We promote a bunyan record.labels property to the LogEntry.labels
property. This enables label based filtering to work auto-magically.

However, this can be problematic when the labels are not type correct
as per the logging proto requirements - specifically, the labels values
have to be strings.

Restrict the automatic promotion to happen only in the cases when
know that the labels are well-formatted.

Fixes: #265

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 21, 2019
@ofrobots ofrobots added the semver: patch A minor bug fix or small change. label Feb 21, 2019
We promote a bunyan record.labels property to the LogEntry.labels
property. This enables label based filtering to work auto-magically.

However, this can be problematic when the labels are not type correct
as per the logging proto requirements - specifically, the labels values
have to be strings.

Restrict the automatic promotion to happen only in the cases when
know that the labels are well-formatted.

Fixes: #265
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 21, 2019

Codecov Report

Merging #266 into master will increase coverage by 3.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #266      +/-   ##
==========================================
+ Coverage   91.78%   94.87%   +3.09%     
==========================================
  Files           2        2              
  Lines          73       78       +5     
  Branches        6        8       +2     
==========================================
+ Hits           67       74       +7     
+ Misses          6        4       -2
Impacted Files Coverage Δ
src/index.ts 96.61% <100%> (+4.01%) ⬆️

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 eeee4ed...8d90a8a. Read the comment docs.

Copy link
Copy Markdown
Contributor

@DominicKramer DominicKramer left a comment

Choose a reason for hiding this comment

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

LGTM once tests pass.

@ofrobots
Copy link
Copy Markdown
Contributor Author

System tests had failed with

Error: Quota exceeded for quota metric 'clouderrorreporting.googleapis.com/manage_error_events_data_requests' and limit 'ManageErrorEventsDataRequestsPerMinutePerUser' of service 'clouderrorreporting.googleapis.com' for consumer 'project_number:1046198160504'.

@ofrobots ofrobots added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 21, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 21, 2019
@ofrobots
Copy link
Copy Markdown
Contributor Author

/cc @JustinBeckwith could we get the error reporting quota increased for the system test project? I added a few patches which meant that the tests ran a few times, probably pushing us over the quota. It would be good if we were not too close to the edge.

@JustinBeckwith
Copy link
Copy Markdown
Contributor

Was this a logging quota or error reporting API quota?

@ofrobots
Copy link
Copy Markdown
Contributor Author

Error reporting.

@ofrobots ofrobots added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 23, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 23, 2019
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Feb 28, 2019
@ofrobots ofrobots added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 1, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 1, 2019
@ofrobots ofrobots added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 1, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 1, 2019
@ofrobots ofrobots merged commit 0a2f379 into master Mar 1, 2019
@ofrobots ofrobots deleted the fix-265 branch March 1, 2019 06:58
@yoshi-automation yoshi-automation removed the 🚨 This issue needs some love. label Apr 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement. semver: patch A minor bug fix or small change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

use of record field "labels" causes unhandled exception TypeError: .google.logging.v2.LogEntry.labels: object expected

6 participants