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.

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

initial cut at schema compliant with FHIR AuditEvent #12

Merged
merged 8 commits into from
Feb 7, 2025

Conversation

pbugni
Copy link
Contributor

@pbugni pbugni commented Jan 9, 2025

No description provided.

@pbugni pbugni requested review from mcjustin and ivan-c January 28, 2025 00:49
Copy link
Member

@mcjustin mcjustin left a comment

Choose a reason for hiding this comment

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

It's a bit change, but I really like this direction and we probably have just enough time to square w/ it for Let's Talk Tech (going live next week).
Can you please add an example to help disambiguate action from code?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link

@daniellrgn daniellrgn left a comment

Choose a reason for hiding this comment

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

Good move in my eyes. For unspecified information that might be necessary to log (ip address, user agent, session id) would we extend this schema at the top level, or would we expect additional data elements in one of the detail fields?
Not being super familiar working with the old schema, was its tag support not useful, too inconsistent, or is its function just adequately replaced by other fields in the new schema?

README.md Outdated
Comment on lines 41 to 46
- `category`: major type of the event, such as:
- `authentication`: events related to login or authentication.
- `authorization`: events related to access control or permissions changes.
- `security`: general security-related events.
- `data-access`: events where healthcare data is accessed or modified.
- `configuration`: events involving system or configuration changes.

Choose a reason for hiding this comment

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

Would we just omit if the logged event doesn't fit into these five buckets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we opted to do away with category as it's generally captured by action

README.md Outdated
Comment on lines 55 to 56
- `patient`: the **subject** of the activity, i.e. `Patient/ab-123-ef`
- `agent`: actor involved in the event, i.e. `Practitioner/123-abc`

Choose a reason for hiding this comment

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

There was a brief discussion last week around the need for field type consistency for ELK to index records properly, and user/subject came up as one that sometimes may be easier to include as a string (id) but other times may be included as an object (with additional user information, secondary IDs, etc.), and some systems may have access to only one or different identifiers than others for the same user. With this schema, would we move toward simplifying to the most appropriate string for the system, or would we fudge the FHIR a bit and include some info as a more complex data type?

Copy link
Contributor Author

@pbugni pbugni Jan 29, 2025

Choose a reason for hiding this comment

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

a hybrid, if you will. consistent FHIR reference strings where possible. i.e. <ResourceType>/<primary_key>

README.md Outdated Show resolved Hide resolved
Copy link
Member

@ivan-c ivan-c left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!!

README.md Outdated Show resolved Hide resolved
@pbugni pbugni marked this pull request as ready for review January 29, 2025 00:12
Copy link

@daniellrgn daniellrgn left a comment

Choose a reason for hiding this comment

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

Thanks Paul!

Copy link
Member

@mcjustin mcjustin left a comment

Choose a reason for hiding this comment

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

@pbugni
Copy link
Contributor Author

pbugni commented Feb 7, 2025

@ivan-c please feel free to add review comments - i can create another PR if necessary. For now, probably best to merge this for easier reference by others.

@pbugni pbugni merged commit c28102f into develop Feb 7, 2025
@pbugni pbugni deleted the feature/docs-audit-event branch February 7, 2025 00:26
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.

4 participants