perf!: getEntries filters 24 hour timestamp by default#955
perf!: getEntries filters 24 hour timestamp by default#955freelerobot merged 8 commits intomasterfrom
Conversation
google-cla
bot
added
the
cla: yes
product-auto-label
bot
added
the
api: logging
freelerobot
added
the
priority: p2
Codecov Report
@@ Coverage Diff @@
## master #955 +/- ##
=======================================
Coverage 98.38% 98.39%
=======================================
Files 18 18
Lines 12977 12985 +8
Branches 412 414 +2
=======================================
+ Hits 12768 12776 +8
Misses 207 207
Partials 2 2
Continue to review full report at Codecov.
|
freelerobot
changed the title
bcoe
left a comment
bcoe
left a comment
There was a problem hiding this comment.
Unless this is changing the API surface, I'd avoid indicating that it's a breaking change with !, as this will result in a major version bump.
freelerobot
changed the title
freelerobot
changed the title
|
@bcoe possible to resolve the change request so this one can be merged as well? |
| // By default, sort entries by descending timestamp | ||
| let reqOpts = extend({orderBy: 'timestamp desc'}, options); | ||
|
|
||
| // By default, filter entries to last 24 hours only |
There was a problem hiding this comment.
What was the previous behavior?
There was a problem hiding this comment.
Previously, it would get as many entries as possible until a max threshold was reached. It took about 10 minutes. The default behavior across CLI is actually to filter by 24 hours, as well as what's recommended in documentation.
We were getting a lot of support tickets across different language repos on this.
This fix reduces query time to a few seconds.
There was a problem hiding this comment.
Sounds good, I just wanted to get that information to determine if it's a breaking change, which it sounds like it is. What do you think?
There was a problem hiding this comment.
Great question, this actually got a lot of debate. After some back and forth in the chats, we decided to mark it as breaking. and bundle it with another breaking fix that just got merged last night
3 participants
Fixes #924
This fix/feat is applied across client libraries in Go, Python, Java and Node, e.g.
googleapis/google-cloud-go#3120
googleapis/python-logging#73