Add Additional Logging for new endpoint #3779

tyler-spangler6 · 2024-11-20T21:31:03Z

What was the problem?

This adds a new field to logging and updates an existing field for only the new expanded classifier.

Previous Logging:

{
    "vagov_claim_id": 100,
    "claim_type": "new",
    "classification_code": 8989,
    "classification_name": "Mental Disorders",
    "contention_text": "PTSD (post-traumatic stress disorder)",
    "diagnostic_code": "None",
    "is_in_dropdown": true,
    "is_lookup_table_match": true,
    "is_multi_contention": true,
    "endpoint": "/va-gov-claim-classifier",
    "date": "2024-11-20 14:20:24",
    "level": "info"
}
{
    "vagov_claim_id": 100,
    "claim_type": "claim_for_increase",
    "classification_code": 8940,
    "classification_name": "Cancer - Musculoskeletal - Other",
    "contention_text": "unmapped contention text",
    "diagnostic_code": 5012,
    "is_in_dropdown": false,
    "is_lookup_table_match": true,
    "is_multi_contention": true,
    "endpoint": "/va-gov-claim-classifier",
    "date": "2024-11-20 14:20:24",
    "level": "info"
}
{
    "vagov_claim_id": 100,
    "claim_type": "new",
    "classification_code": null,
    "classification_name": null,
    "contention_text": "unmapped contention text",
    "diagnostic_code": 8550,
    "is_in_dropdown": false,
    "is_lookup_table_match": false,
    "is_multi_contention": true,
    "endpoint": "/va-gov-claim-classifier",
    "date": "2024-11-20 14:20:24",
    "level": "info"
}
{
    "claim_id": 100,
    "form526_submission_id": 500,
    "is_fully_classified": false,
    "percent_clasified": 66.66666666666666,
    "num_processed_contentions": 3,
    "num_classified_contentions": 2,
    "endpoint": "/va-gov-claim-classifier",
    "date": "2024-11-20 14:20:24",
    "level": "info"
}
{
    "process_time": 0.004019021987915039,
    "url": "/va-gov-claim-classifier",
    "date": "2024-11-20 14:20:24",
    "level": "info"
}

New Logging for expanded classifier:

{
    "vagov_claim_id": 100,
    "claim_type": "new",
    "classification_code": 8989,
    "classification_name": "Mental Disorders",
    "contention_text": "PTSD (post-traumatic stress disorder)",
    "processed_contention_text": "ptsd post traumatic stress disorder",
    "diagnostic_code": "None",
    "is_in_dropdown": true,
    "is_lookup_table_match": true,
    "is_multi_contention": true,
    "endpoint": "/expanded-contention-classification",
    "date": "2024-11-20 14:27:09",
    "level": "info"
}
{
    "vagov_claim_id": 100,
    "claim_type": "claim_for_increase",
    "classification_code": 8940,
    "classification_name": "Cancer - Musculoskeletal - Other",
    "contention_text": "unmapped contention text ['']",
    "processed_contention_text": "",
    "diagnostic_code": 5012,
    "is_in_dropdown": false,
    "is_lookup_table_match": true,
    "is_multi_contention": true,
    "endpoint": "/expanded-contention-classification",
    "date": "2024-11-20 14:27:09",
    "level": "info"
}
{
    "vagov_claim_id": 100,
    "claim_type": "new",
    "classification_code": 8997,
    "classification_name": "Musculoskeletal - Knee",
    "contention_text": "unmapped contention text ['acl tear']",
    "processed_contention_text": "acl tear",
    "diagnostic_code": 8550,
    "is_in_dropdown": false,
    "is_lookup_table_match": true,
    "is_multi_contention": true,
    "endpoint": "/expanded-contention-classification",
    "date": "2024-11-20 14:27:09",
    "level": "info"
}
{
    "claim_id": 100,
    "form526_submission_id": 500,
    "is_fully_classified": true,
    "percent_clasified": 100.0,
    "num_processed_contentions": 3,
    "num_classified_contentions": 3,
    "endpoint": "/expanded-contention-classification",
    "date": "2024-11-20 14:27:09",
    "level": "info"
}
{
    "process_time": 0.004132986068725586,
    "url": "/expanded-contention-classification",
    "date": "2024-11-20 14:27:09",
    "level": "info"
}

The additional field of processed_classified_text will us compare classifications based on the text processing lookup. This should also prevent logging any PII because we will only log the contention text if there is a match with the lookup table after processing. Additionally adding the processed text in the contention text field will help with our DataDog widgets to differentiate between unclassifiable unmapped text and classifiable unmapped text.

Associated tickets or Slack threads:

https://github.com/orgs/department-of-veterans-affairs/projects/873/views/2?pane=issue&itemId=88018308&issue=department-of-veterans-affairs%7Cvagov-claim-classification%7C643

How does this fix it?¹

How to test this PR

Step 1
Step 2²

Pull-Requests guidelines. If PR is significant, update Current Software State wiki page. ↩
To check if a PR will succeed in the SecRel workflow, test PRs in the SecRel pipeline. ↩

…t only

github-actions · 2024-11-20T21:35:35Z

Test Results

136 tests ±0 136 ✅ ±0 22s ⏱️ -3s
39 suites ±0 0 💤 ±0
39 files ±0 0 ❌ ±0

Results for commit f245737. ± Comparison against base commit 89bbf65.

♻️ This comment has been updated with latest results.

github-actions · 2024-11-20T21:35:52Z

JaCoCo Test Coverage

Overall Project	72.23%	❌

There is no coverage information present for the Files changed

williamphelps13

Nice work here! Just one comment - Could the new code in log_contention_stats be broken into its own helper function?

williamphelps13 · 2024-11-27T19:00:37Z

domain-cc/cc-app/src/python_src/api.py

+    if request.url.path == "/expanded-contention-classification":
+        processed_text = expanded_lookup_table.prep_incoming_text(
+            contention.contention_text
+        )
+        # only log these items if the expanded lookup returns a classification code
+        if expanded_lookup_table.get(contention.contention_text)["classification_code"]:
+            if log_contention_text == "unmapped contention text":
+                log_contention_text = f"unmapped contention text {[processed_text]}"
+
+            logging_dict.update(
+                {
+                    "processed_contention_text": processed_text,
+                    "contention_text": log_contention_text,
+                }
+            )
+        # log none as the processed text if it is not in the LUT and leave unmapped contention text as is
+        else:
+            logging_dict.update({"processed_contention_text": None})
+        log_as_json(logging_dict)
+    else:
+        log_as_json(logging_dict)


What if broke this code out into a helper function? Something like:

def _update_logging_for_expanded_contention( logging_dict: dict, contention_text: str, log_contention_text: str ): """ # Updates the logging dictionary for `/expanded-contention-classification` """ processed_text = expanded_lookup_table.prep_incoming_text(contention_text) classification = expanded_lookup_table.get(contention_text) if classification.get("classification_code"): if log_contention_text == "unmapped contention text": log_contention_text = f"unmapped contention text {[processed_text]}" logging_dict.update( { "processed_contention_text": processed_text, "contention_text": log_contention_text, } ) # log none as the processed text if it is not in the LUT and leave unmapped contention text as is else: logging_dict.update({"processed_contention_text": None}) return logging_dict

That is then used like:

if request.url.path == "/expanded-contention-classification": logging_dict = _update_logging_for_expanded_contention( logging_dict, contention_text, log_contention_text ) log_as_json(logging_dict)

williamphelps13

Thanks for checking out the comment! Glad it looked like a positive change. This PR looks good to me.

…fier

gabezurita

LGTM!

gabezurita · 2024-12-02T18:46:10Z

domain-cc/cc-app/src/python_src/api.py

+        )
+
+        # log_as_json(logging_dict)
+    # else:


Is this commented-out code left in deliberately?

@gabezurita Sorry I missed this comment before merging. That was accidentally left behind, I will clean this up in a following PR.

add logs to contention level logs for expanded classification endpoin…

8a7d205

…t only

tyler-spangler6 added 2 commits November 21, 2024 08:02

adjusted prep_incoming_text and get, added tests

a00eb02

consolidated logging and added conditionals to prevent unwanted logging

4781540

tyler-spangler6 marked this pull request as ready for review November 22, 2024 14:33

tyler-spangler6 requested a review from a team as a code owner November 22, 2024 14:33

adds tests for logging processed text

0cf8724

williamphelps13 reviewed Nov 27, 2024

View reviewed changes

broke out code into helper function

2e77960

williamphelps13 self-requested a review December 2, 2024 17:36

williamphelps13 approved these changes Dec 2, 2024

View reviewed changes

tyler-spangler6 added 2 commits December 2, 2024 10:46

Merge branch 'develop' into conditions-update-logging-expanded-classi…

6fda605

…fier

Merge branch 'develop' into conditions-update-logging-expanded-classi…

f245737

…fier

raudabaugh approved these changes Dec 2, 2024

View reviewed changes

tyler-spangler6 merged commit 9164fd1 into develop Dec 2, 2024
16 checks passed

tyler-spangler6 deleted the conditions-update-logging-expanded-classifier branch December 2, 2024 18:53

gabezurita approved these changes Dec 2, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Additional Logging for new endpoint #3779

Add Additional Logging for new endpoint #3779

tyler-spangler6 commented Nov 20, 2024

github-actions bot commented Nov 20, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024 •

edited

Loading

williamphelps13 left a comment

williamphelps13 Nov 27, 2024

williamphelps13 left a comment

gabezurita left a comment

gabezurita Dec 2, 2024

tyler-spangler6 Dec 2, 2024

Add Additional Logging for new endpoint #3779

Add Additional Logging for new endpoint #3779

Conversation

tyler-spangler6 commented Nov 20, 2024

What was the problem?

How does this fix it?1

How to test this PR

Footnotes

github-actions bot commented Nov 20, 2024 • edited Loading

Test Results

github-actions bot commented Nov 20, 2024 • edited Loading

JaCoCo Test Coverage

williamphelps13 left a comment

Choose a reason for hiding this comment

williamphelps13 Nov 27, 2024

Choose a reason for hiding this comment

williamphelps13 left a comment

Choose a reason for hiding this comment

gabezurita left a comment

Choose a reason for hiding this comment

gabezurita Dec 2, 2024

Choose a reason for hiding this comment

tyler-spangler6 Dec 2, 2024

Choose a reason for hiding this comment

How does this fix it?¹

github-actions bot commented Nov 20, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024 •

edited

Loading