Add cost observability BQ table [VS-441] #7891

mcovarr · 2022-06-13T15:03:35Z

No description provided.

codecov · 2022-06-13T15:30:44Z

Codecov Report

❗ No coverage uploaded for pull request base (ah_var_store@727c1da). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             ah_var_store     #7891   +/-   ##
================================================
  Coverage                ?   86.290%           
  Complexity              ?     35195           
================================================
  Files                   ?      2170           
  Lines                   ?    164888           
  Branches                ?     17785           
================================================
  Hits                    ?    142282           
  Misses                  ?     16281           
  Partials                ?      6325

rsasch

LGTM 👍🏻

RoriCremer · 2022-06-14T19:19:10Z

scripts/variantstore/wdl/GvsAssignIds.wdl

@@ -19,7 +19,14 @@ workflow GvsAssignIds {
  String sample_info_table = "sample_info"
  String sample_info_schema_json = '[{"name": "sample_name","type": "STRING","mode": "REQUIRED"},{"name": "sample_id","type": "INTEGER","mode": "NULLABLE"},{"name":"is_loaded","type":"BOOLEAN","mode":"NULLABLE"},{"name":"is_control","type":"BOOLEAN","mode":"REQUIRED"},{"name":"withdrawn","type":"TIMESTAMP","mode":"NULLABLE"}]'
  String sample_load_status_json = '[{"name": "sample_id","type": "INTEGER","mode": "REQUIRED"},{"name":"status","type":"STRING","mode":"REQUIRED"}, {"name":"event_timestamp","type":"TIMESTAMP","mode":"REQUIRED"}]'
-
+  String cost_observability_json = '[ { "name": "call_set_identifier", "type": "STRING", "mode": "REQUIRED" }, ' +


ok not for this pr---but I truly do think some of these would benefit from a description

sure I can do that now

RoriCremer · 2022-06-14T19:20:34Z

scripts/variantstore/wdl/GvsAssignIds.wdl

+      datatype = "cost_observability",
+      schema_json = cost_observability_json,
+      max_table_id = 1,
+      superpartitioned = "false",


what is superpartitioned!??!?!

and I think this should be partitioned on job_start_timestamp, but I could be persuaded otherwise

or maybe on call set identifier?

Yeah, that is great too

We used to only be able to partition on #s (which is one of the reasons we have GvsAssignIds) and it was originally developed for querying logs by date, so I think I'm stuck in the past w my recs

superpartitioning is the thing we do when we create a ref ranges or vet table for every 4000 samples to get around the BQ partitioning limits.

actually the CreateTables code is not flexible on the partition key so I'm going to make a larger revision here...

mcovarr · 2022-06-14T21:11:42Z

asking for a re-review since I ended up changing a lot...

rsasch

too bad you couldn't re-use the existing task to create the table, but LGTM 👍🏻

RoriCremer · 2022-06-15T19:31:04Z

scripts/variantstore/wdl/GvsAssignIds.wdl

+                                 '  { "name": "call_start_timestamp", "type": "TIMESTAMP", "mode": "REQUIRED" }, ' + # When the call logging this event was started
+                                 '  { "name": "event_timestamp", "type": "TIMESTAMP", "mode": "REQUIRED" }, '      + # When the observability event was logged
+                                 '  { "name": "event_key", "type": "STRING", "mode": "REQUIRED" }, '               + # The type of observability event being logged
+                                 '  { "name": "event_bytes", "type": "INTEGER", "mode": "REQUIRED" } ] '             # Number of bytes reported for this observability event


hahaha I meant that the "description" could go in the BQ table, but this works perfectly too

oh I forgot that was even a thing we could do 🤦

RoriCremer · 2022-06-15T19:35:57Z

scripts/variantstore/wdl/GvsAssignIds.wdl

+    fi
+  >>>
+  runtime {
+    docker: "us.gcr.io/broad-dsde-methods/variantstore:ah_var_store_2022_05_16"


why are we using our custom docker?

I'm worried about this getting stale (though I suppose it wont matter for this use case)

I feel like we tend to use: docker: "us.gcr.io/broad-gatk/gatk:4.1.7.0"

RoriCremer · 2022-06-15T19:40:05Z

scripts/variantstore/wdl/GvsAssignIds.wdl

+  String cost_observability_json =
+                                 '[ { "name": "call_set_identifier", "type": "STRING", "mode": "REQUIRED" }, '     + # The name by which we refer to the callset
+                                 '  { "name": "step", "type": "STRING", "mode": "REQUIRED" }, '                    + # The name of the core GVS workflow to which this belongs
+                                 '  { "name": "call", "type": "STRING", "mode": "NULLABLE" }, '                    + # The WDL call to which this belongs
+                                 '  { "name": "shard_identifier", "type": "STRING", "mode": "NULLABLE" }, '        + # A unique identifier for this shard, may or may not be its index
+                                 '  { "name": "call_start_timestamp", "type": "TIMESTAMP", "mode": "REQUIRED" }, ' + # When the call logging this event was started
+                                 '  { "name": "event_timestamp", "type": "TIMESTAMP", "mode": "REQUIRED" }, '      + # When the observability event was logged
+                                 '  { "name": "event_key", "type": "STRING", "mode": "REQUIRED" }, '               + # The type of observability event being logged
+                                 '  { "name": "event_bytes", "type": "INTEGER", "mode": "REQUIRED" } ] '             # Number of bytes reported for this observability event


Suggested change

String cost_observability_json =

'[ { "name": "call_set_identifier", "type": "STRING", "mode": "REQUIRED" }, ' + # The name by which we refer to the callset

' { "name": "step", "type": "STRING", "mode": "REQUIRED" }, ' + # The name of the core GVS workflow to which this belongs

' { "name": "call", "type": "STRING", "mode": "NULLABLE" }, ' + # The WDL call to which this belongs

' { "name": "shard_identifier", "type": "STRING", "mode": "NULLABLE" }, ' + # A unique identifier for this shard, may or may not be its index

' { "name": "call_start_timestamp", "type": "TIMESTAMP", "mode": "REQUIRED" }, ' + # When the call logging this event was started

' { "name": "event_timestamp", "type": "TIMESTAMP", "mode": "REQUIRED" }, ' + # When the observability event was logged

' { "name": "event_key", "type": "STRING", "mode": "REQUIRED" }, ' + # The type of observability event being logged

' { "name": "event_bytes", "type": "INTEGER", "mode": "REQUIRED" } ] ' # Number of bytes reported for this observability event

String cost_observability_json =

'[ { "name": "call_set_identifier", "type": "STRING", "mode": "REQUIRED", "description": "The name by which we refer to the callset" }, ' +

' { "name": "step", "type": "STRING", "mode": "REQUIRED", "description": "The name of the core GVS workflow to which this belongs" }, ' +

' { "name": "call", "type": "STRING", "mode": "NULLABLE", "description": "The WDL call to which this belongs" }, ' +

' { "name": "shard_identifier", "type": "STRING", "mode": "NULLABLE" , "description": "A unique identifier for this shard, may or may not be its index" }, ' +

' { "name": "call_start_timestamp", "type": "TIMESTAMP", "mode": "REQUIRED", "description": "When the call logging this event was started" }, ' +

' { "name": "event_timestamp", "type": "TIMESTAMP", "mode": "REQUIRED", "description": "When the observability event was logged"}, ' +

' { "name": "event_key", "type": "STRING", "mode": "REQUIRED", "description": "The type of observability event being logged" }, ' +

' { "name": "event_bytes", "type": "INTEGER", "mode": "REQUIRED", "description": "Number of bytes reported for this observability event" } ] '

mcovarr added 2 commits June 13, 2022 09:52

cost observability

3fd34b8

oops

ba57f1a

mcovarr requested review from rsasch and RoriCremer June 13, 2022 15:03

rsasch approved these changes Jun 13, 2022

View reviewed changes

mcovarr force-pushed the vs_441_cost_observability branch from c00ff15 to ba57f1a Compare June 14, 2022 10:47

RoriCremer reviewed Jun 14, 2022

View reviewed changes

RoriCremer approved these changes Jun 14, 2022

View reviewed changes

RoriCremer reviewed Jun 14, 2022

View reviewed changes

mcovarr added 4 commits June 14, 2022 15:58

PR feedback

1636157

PR feedback

3400e5d

oops

673f77e

gah

b15853b

mcovarr requested review from RoriCremer and rsasch June 14, 2022 21:11

rsasch approved these changes Jun 14, 2022

View reviewed changes

RoriCremer reviewed Jun 15, 2022

View reviewed changes

RoriCremer approved these changes Jun 15, 2022

View reviewed changes

mcovarr merged commit 89a7d5d into ah_var_store Jun 15, 2022

mcovarr deleted the vs_441_cost_observability branch June 15, 2022 19:33

RoriCremer reviewed Jun 15, 2022

View reviewed changes

RoriCremer pushed a commit that referenced this pull request Jun 29, 2022

Add cost observability BQ table [VS-441] (#7891)

18ce129

This was referenced Mar 17, 2023

lb merge gvs branch #8248

Closed

testing something, please ignore #8251

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cost observability BQ table [VS-441] #7891

Add cost observability BQ table [VS-441] #7891

mcovarr commented Jun 13, 2022

codecov bot commented Jun 13, 2022 •

edited

Loading

rsasch left a comment

RoriCremer Jun 14, 2022

mcovarr Jun 14, 2022

RoriCremer Jun 14, 2022

mcovarr Jun 14, 2022

RoriCremer Jun 14, 2022

RoriCremer Jun 14, 2022

mcovarr Jun 14, 2022

mcovarr Jun 14, 2022

mcovarr commented Jun 14, 2022

rsasch left a comment

RoriCremer Jun 15, 2022

mcovarr Jun 15, 2022

RoriCremer Jun 15, 2022

RoriCremer Jun 15, 2022

Add cost observability BQ table [VS-441] #7891

Add cost observability BQ table [VS-441] #7891

Conversation

mcovarr commented Jun 13, 2022

codecov bot commented Jun 13, 2022 • edited Loading

Codecov Report

rsasch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcovarr commented Jun 14, 2022

rsasch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 13, 2022 •

edited

Loading