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.

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 getLogs() and getLogsStream() #692

Merged
merged 4 commits into from
Jan 31, 2020

Conversation

stephenplusplus
Copy link
Contributor

@stephenplusplus stephenplusplus commented Jan 30, 2020

RE: #550

This adds a new method, logging.getLogs() and logging.getLogsStream() using the logs.list API: https://cloud.google.com/logging/docs/reference/v2/rest/v2/logs/list

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 30, 2020
@@ -97,6 +98,20 @@ describe('Logging', () => {
await getAndDelete(logging.getSinks.bind(logging));
}

async function deleteLogs() {
const [logs] = await logging.getLogs();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using getLogs() and a naming convention consistent with the other test resources, we can clean up all test logs that are matched after each test run.

@codecov
Copy link

codecov bot commented Jan 30, 2020

Codecov Report

Merging #692 into master will increase coverage by 0.56%.
The diff coverage is 91.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #692      +/-   ##
==========================================
+ Coverage   68.24%   68.81%   +0.56%     
==========================================
  Files          36       36              
  Lines       10604    10828     +224     
  Branches      174      189      +15     
==========================================
+ Hits         7237     7451     +214     
- Misses       3366     3376      +10     
  Partials        1        1
Impacted Files Coverage Δ
samples/logs.js 60.61% <10%> (-3.73%) ⬇️
src/index.ts 99.55% <99.02%> (-0.1%) ⬇️
src/sink.ts 98.86% <0%> (+0.56%) ⬆️
src/v2/logging_service_v2_client.js 94.02% <0%> (+0.64%) ⬆️

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 a8d587f...201c8cb. Read the comment docs.

Copy link
Contributor

@JustinBeckwith JustinBeckwith left a comment

Choose a reason for hiding this comment

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

LGTM, but please let @bcoe have a look too.

async function listLogs() {
// [START logging_list_logs]
// Imports the Google Cloud client library
const {Logging} = require('@google-cloud/logging');
Copy link
Contributor

Choose a reason for hiding this comment

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

If this got embedded in cloud.google.com, the code inside of the region tag would have a top level await. That means folks couldn't reasonably copy and paste the sample without making changes. Instead, can we follow a model that introduces an async wrapper method? For Example:

async function listLogsMain() {
  // [START logging_list_logs]
  // Imports the Google Cloud client library
  const {Logging} = require('@google-cloud/logging');
  
  // Creates a client
  const logging = new Logging();

  async function listLogs() {
    const [logs] = await logging.getLogs();
    console.log('Logs:');
    logs.forEach(log => {
      console.log(log.name);
    });
  }
  listLogs();
  // [END logging_list_logs]
}

It does leave a hanging promise, but at least this way the sample is copy/pastable.

@stephenplusplus stephenplusplus added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 31, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 31, 2020
@stephenplusplus stephenplusplus merged commit d582eeb into googleapis:master Jan 31, 2020
@stephenplusplus stephenplusplus deleted the spp--getLogs branch January 31, 2020 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants