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.

Sign up for GitHub

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

Jump to bottom

Fix cost (for methods cromwell), add explicit example #278

Merged
merged 5 commits into from
Apr 2, 2024
Merged

Fix cost (for methods cromwell), add explicit example #278

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a08eaaa
Fix cost (for methods cromwell), add explicit example
sjfleming Jan 19, 2024
f1344e2
Merge branch 'main' into sf_cost
bshifaw Mar 18, 2024
438433f
Update cost unit test to query export_time
sjfleming Mar 29, 2024
dd7dabf
One more that I missed
sjfleming Mar 29, 2024
4cefaec
Modify README for cost
sjfleming Mar 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,15 @@ Cromshell is a CLI for submitting workflows to a Cromwell server and monitoring/
* Requires the `bq_cost_table` key to exist in the cromshell
configuration file and have a value equal to the BigQuery cost table
for your GCP billing project.
* For example, using the Methods cromwell server, your `~/.cromshell/cromshell_config.json`
should be as follows:
```json
{
"cromwell_server": "https://cromwell-v77.dsde-methods.broadinstitute.org/",
"requests_timeout": 5,
"bq_cost_table": "Methods_billing_dump.gcp_billing_export_v1_009C7D_923007_219A6F"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer we hide these values from the open internet, just in case...

Copy link
Contributor

Choose a reason for hiding this comment

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

so how about the following:
"<bq_dataset_hosting_the_billing_datatable>.<the_billing_datatable>"

Copy link
Member Author

Choose a reason for hiding this comment

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

We could do that if we need to yeah

Copy link
Member Author

Choose a reason for hiding this comment

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

(But if there's no risk in exposing these, I'd like the example to be 100% explicit. Because I never would have foudn those values.
So... is there a risk?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Or... better... if we included directions for how to find that table.

Copy link
Member Author

Choose a reason for hiding this comment

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

But I don't know what those directions are

}
```
* `-c/--color` Color outliers in task level cost results.
* `-d/--detailed` Get the cost for a workflow at the task level.

Expand Down
4 changes: 2 additions & 2 deletions src/cromshell/cost/command.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,15 +182,15 @@ def create_bq_query(detailed: bool, bq_cost_table: str) -> str:
AND task.key LIKE "wdl-task-name"
AND wfid.key LIKE "cromwell-workflow-id"
AND wfid.value like @workflow_id
AND partition_time BETWEEN @start_date AND @end_date
AND export_time BETWEEN @start_date AND @end_date
Copy link
Contributor

Choose a reason for hiding this comment

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

I looked at our cost tabe and do see this key in it, instead of the partition_time key. But I wonder why it was partition_time at the beginning, starting from the shell version. Did Google change things underneath us?

export_time is documented here, but not partition_time
https://cloud.google.com/billing/docs/how-to/export-data-bigquery-tables/detailed-usage

And confusing the situation, even more, are the two other *time values: usage_start_time and usage_end_time. And now I wonder, should we use them instead of export_time?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good questions. I do remember some email about google changing things. I think this is what happened, but I can't verify it.

Not sure about usage_end_time versus export_time

GROUP BY 1,2,3
ORDER BY 4 DESC
"""
else:
return f"""
SELECT sum(cost) as cost
FROM {bq_cost_table}, UNNEST(labels)
WHERE value LIKE @workflow_id AND partition_time BETWEEN @start_date AND @end_date
WHERE value LIKE @workflow_id AND export_time BETWEEN @start_date AND @end_date
"""


Expand Down
Loading