Skip to content

Sequencing monitoring#5

Open
RyandenOtter wants to merge 97 commits intosequencing-developfrom
sequencing-monitoring
Open

Sequencing monitoring#5
RyandenOtter wants to merge 97 commits intosequencing-developfrom
sequencing-monitoring

Conversation

@RyandenOtter
Copy link
Collaborator

Here are the updates for the monitoring views.

Jacob Ferriero and others added 30 commits November 23, 2020 15:38
... on multiple projects

fixup!
Add an environment variable to support overriding the default project
for the BigQuery Client. By default this will be the project in which
the cloud function is deployed.
Change the tests to use new utils module.
Move utility methods into a utils module
* Restructures code into constants and exception modules
* Implements Backlog Publisher / Subscriber algorithm for ordering incrementals
* Implements basic integration tests for Publisher / Subscriber
@jaketf
Copy link
Owner

jaketf commented Jan 13, 2021

you need to sign the first two commits.

git commit --amend --author="Author Name <email@address.com>"

Copy link
Owner

@jaketf jaketf left a comment

Choose a reason for hiding this comment

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

I'd like if the SQL had more structured place holders for dataset / project ids and a script to render them.
For example you could use envsubst
This way we could render them and dry run them in the cloud build so we'd have some basic level of CI on these queries.

Adding the views and tables for ingest monitoring

Remove unneeded sql

Update README.md

Fix formatting issues on the tables.
@jaketf jaketf force-pushed the sequencing-monitoring branch from f8c6ed9 to 41ebdbd Compare January 13, 2021 00:46
RyandenOtter and others added 10 commits January 13, 2021 07:10
…/README.md


Re-Wording

Co-authored-by: Jacob Ferriero <jferriero@google.com>
…/README.md

Co-authored-by: Jacob Ferriero <jferriero@google.com>
…/README.md

Co-authored-by: Jacob Ferriero <jferriero@google.com>
…/README.md

Co-authored-by: Jacob Ferriero <jferriero@google.com>
…/README.md

Co-authored-by: Jacob Ferriero <jferriero@google.com>
Adding additional information to the READMEmd
RyandenOtter and others added 6 commits January 14, 2021 14:04
Co-authored-by: Jacob Ferriero <jferriero@google.com>
…ingest/test_monitoring_tables.sh

Co-authored-by: Jacob Ferriero <jferriero@google.com>
Move the monitoring.sql files up a directory
Split out the monitoring sql into its own files (Note the numbering is to ensure the files are listed in the correct order because they are all dependent on one another)

test_monitoring_tables.sh
Made some alterations to test_monitoring_tables.sh to ensure dataset gets cleaned up even if the sqls error.
removed the dry run argument from the bq query to ensure the sql is actually applied.

Readme:
Moved the Monitoring section out to the higher-level README

cloudbuild.yaml
Updated the cloudbuild to point to the new location of the tests.
Fix 03_gcf_ingest_latest_by_table.sql

Some more README fixes.
@RyandenOtter
Copy link
Collaborator Author

RyandenOtter commented Jan 23, 2021

Hey! It looks like I don't have permissions to delete datasets in your test project, which is causing my local tests to fail.
It also looks like the gcs_event_based_ingest_ci image doesn't have envsubst installed, which is causing the build to fail.

@@ -0,0 +1,28 @@
/*
Copy link
Owner

Choose a reason for hiding this comment

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

is there a reasoning for the numbering 01, 02, 03 in these query filenames? can we remove that?

Copy link
Owner

Choose a reason for hiding this comment

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

I see you need to run these sql in a certain order because they create metadata entries that you depend on existing and you're doing this with sort. If there's a way to just have an ordered list in the shell script that might be better than confusing filenames IMO.

destination_table.project_id,
destination_table.dataset_id,
destination_table.table_id,
start_time; No newline at end of file
Copy link
Owner

Choose a reason for hiding this comment

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

newline

fi
done < <(find monitoring -path "*.sql" | sort)
echo "removing dataset $DATASET"
trap "bq rm -r -f -d $DATASET" EXIT
Copy link
Owner

Choose a reason for hiding this comment

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

nit. this line will never get executed if there is a failure above. Trap statement should be moved to like 29 (see suggestion above).

Suggested change
trap "bq rm -r -f -d $DATASET" EXIT

exit "$result"
fi
done < <(find monitoring -path "*.sql" | sort)
echo "removing dataset $DATASET"
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
echo "removing dataset $DATASET"


export DATASET="test_monitoring_dataset_${BUILD_ID}"
echo "Creating ${DATASET}"
bq --location=US mk -f -d "$DATASET"
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
bq --location=US mk -f -d "$DATASET"
bq --location=US mk -f -d "$DATASET"
trap 'echo "removing dataset $DATASET" && bq rm -r -f -d $DATASET' EXIT

@@ -0,0 +1,41 @@
#!/bin/bash

# Copyright 2019 Google Inc.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2019 Google Inc.
# Copyright 2021 Google Inc.

@@ -0,0 +1,51 @@
/*
# Copyright 2020 Google LLC
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2020 Google LLC
# Copyright 2021 Google LLC

@@ -0,0 +1,57 @@
/*
# Copyright 2020 Google LLC
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2020 Google LLC
# Copyright 2021 Google LLC

@@ -0,0 +1,28 @@
/*
# Copyright 2020 Google LLC
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2020 Google LLC
# Copyright 2021 Google LLC

@jaketf
Copy link
Owner

jaketf commented Feb 4, 2021

You can just install envsubst in Dockerfile.ci

@jaketf jaketf force-pushed the sequencing-develop branch from 21bd43c to 98e7863 Compare March 31, 2021 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments