Repository navigation
BB2-4572/4911/5193: ITS-log initial integration - #1746
JamesDemeryNava wants to merge 78 commits into
Conversation
…evel ones, calculate in log_global_state_metrics
| try: | ||
| result = requests.post( | ||
| 'http://host.docker.internal:8888/v1/log/create', | ||
| headers={'x-api-key': '1234567890123456123456789012345612345678901234561234567890123456'}, |
bwang-icf
left a comment
There was a problem hiding this comment.
was able to run through the validation! thanks for all the hard work on this
| DATABASES_CUSTOM="sqlite:////tmp/db.sqlite3" | ||
| ITS_LOG_API_ENABLED="${ITS_LOG_API_ENABLED}" | ||
| JWKS_PUBLIC_KEY_PEM="${JWKS_PUBLIC_KEY_PEM}" | ||
| CAN_PRIVATE_KEY="${CAN_PRIVATE_KEY}" |
There was a problem hiding this comment.
Why do we need JWKS_PUBLIC_KEY_PEM and CAN_PRIVATE_KEY here?
There was a problem hiding this comment.
Great question, @ryan-morosa added that in his PR, here, not sure why it's showing in this diff.
|
|
||
|
|
||
| def main(): | ||
| files = [ |
There was a problem hiding this comment.
Could also glob for the files in the directory to avoid having to list them manually.
There was a problem hiding this comment.
And perhaps add an option to ignore files that start with _ so we could add etls that we don't want to use yet, if that's needed.
| ETL_CREATION_ENDPOINT = 'http://localhost:8888/v1/etl/create' | ||
| SEQUENCE_CREATION_ENDPOINT = 'http://localhost:8888/v1/sequence/create' | ||
| # Note: This is a dummy API key for local development | ||
| API_KEY = 'abcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefgh' |
There was a problem hiding this comment.
I think we should remove this based on convos
There was a problem hiding this comment.
Need to make sure that BB API container has env vars for ITSLOG_PORT and HOST, as well as the API Key
| 'app_all_successful_client_credentials_call': 'app_successful_client_credentials_call', | ||
| 'app_all_unsuccessful_client_credentials_call': 'app_unsuccessful_client_credentials_call', | ||
| 'app_all_successful_patient_match_call': 'app_successful_patient_match_call', | ||
| 'app_all_unsuccessful_patient_match_call': 'app_unsuccessful_patient_match_call' |
There was a problem hiding this comment.
if these are all the same transformation, could we write the transformation that instead of all of the values?
There was a problem hiding this comment.
Some of them are a little different, but we could either modify those to conform to one pattern or just have a couple of transformation rules. Will work on this today.
There was a problem hiding this comment.
Wound up doing this, just prefixing each global level metric with app_
annamontare-nava
left a comment
There was a problem hiding this comment.
A few more comments but looking good!
| "value": str(count), | ||
| "count": 1, | ||
| }) | ||
| for row in returning_summary_rows: |
There was a problem hiding this comment.
probably remove this too
JIRA Ticket:
BB2-4572
BB2-4911
BB2-5193
What Does This PR Do?
This PR implements pinging a logging API, currently known as its-log, for specific Blue Button log types. This is part of a larger effort to implement a new data pipeline that is simpler and easier to maintain. We add an environment variable, ITS_LOG_API_ENABLED, which controls whether or not we ping the its-log API.
There is a Confluence page here with details on how to run its-log. You will need to reference that page in order to test this PR. Be sure to download DBeaver before reviewing this and running its-log.
What Should Reviewers Watch For?
If you're reviewing this PR, please check for these things in particular:
Validation
What Security Implications Does This PR Have?
Please indicate if this PR does any of the following:
security engineer's approval. @sb-DarenDean @sb-benohe checking this because once this is merged and the waffle switch is enabled (which won't happen any time soon, still need to add the terraform code for the new architecture), BB will be pinging a new API to help track metrics data.
Any Migrations?
etc)