Skip to content

Leverage database triggers to generate object change records #14059

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

Closed
jeremystretch opened this issue Oct 17, 2023 · 6 comments
Closed

Leverage database triggers to generate object change records #14059

jeremystretch opened this issue Oct 17, 2023 · 6 comments
Labels
type: feature Introduction of new functionality to the application

Comments

@jeremystretch
Copy link
Member

NetBox version

v3.6.4

Feature type

Change to existing functionality

Proposed functionality

NetBox currently records the creation, modification, and deletion of objects using application-level logic; namely the change_logging() context manager and receiver functions for Django's post_save and post_delete signals. This FR proposes replacing this logic with PostgreSQL triggers using django-pgtrigger. (This is similar in nature to how django-pghistory works, however we'll keep our general-purpose ObjectChange model in place.)

At a high level, this will entail the following:

  1. Remove the ObjectChange creation logic from the handle_changed_object() and handle_deleted_object() signal receivers. (The receivers themselves must remain to handle webhooks and metrics.)
  2. Enable or replicate django-pghistory's context manager to expose request details to the database.
  3. Define the appropriate triggers (TBD) under ChangeLoggingMixin to effect the automatic creation of a new changelog record whenever an object is created, modified, or deleted.

While much more exploration is needed, I believe all the necessary components have been identified.

Use case

Moving the change logging logic from the Python application layer to the database layer has several major benefits:

  1. Provide a much more robust change logging solution
  2. Eliminate the need to manually invoke the change_logging() context manager when working in the Python shell
  3. Reduce request processing overhead
  4. Simplify the core middleware

Database changes

While this implementation would obviously effect the creation of PostgreSQL triggers, I don't predict that any substantial schema changes to existing models will be necessary.

External dependencies

@jeremystretch jeremystretch added type: feature Introduction of new functionality to the application status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Oct 17, 2023
@ITJamie
Copy link
Contributor

ITJamie commented Oct 25, 2023

one thing to consider. It's currently possible to have models that don't have changelog entries.

I recently ended up making an internal plugin for documenting "last seen" timestamps for inventory items. we did not want changelogs created every time we wanted to update the "last seen" time so a custom field was not an option, instead we made a plugin with a relationship model to inventory items insert /update last seen timestamps there.

I would hope by going down this path we can have some kind of flag on models where changelogs should or should not be created

@DanSheps
Copy link
Member

one thing to consider. It's currently possible to have models that don't have changelog entries.

I recently ended up making an internal plugin for documenting "last seen" timestamps for inventory items. we did not want changelogs created every time we wanted to update the "last seen" time so a custom field was not an option, instead we made a plugin with a relationship model to inventory items insert /update last seen timestamps there.

I would hope by going down this path we can have some kind of flag on models where changelogs should or should not be created

It sounds like currently you are ommitting the ObjectChange mixin.

If I am understanding the proposal, the ObjectChange mixin will still exist but now it will instead generate the triggers for each table that needs to be change logged instead of having the middleware intercept the request and process it.

@ITJamie
Copy link
Contributor

ITJamie commented Oct 25, 2023

It sounds like currently you are ommitting the ObjectChange mixin.

correct

If I am understanding the proposal, the ObjectChange mixin will still exist but now it will instead generate the triggers for each table that needs to be change logged instead of having the middleware intercept the request and process it.

Hopefully, otherwise ill have ~15k changelogs every night when airflow inventorys our ssd's and update the last_seen objects

@jeremystretch
Copy link
Member Author

After digging further into this, I'm not sure it's feasible. All the logic for serializing object representations exists in Python; migrating this to a native database function would be challenging if not impossible. It would also preclude models from overriding their own serialization logic.

Copy link
Contributor

github-actions bot commented Feb 5, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Feb 5, 2024
@jeremystretch
Copy link
Member Author

We might revisit this idea in the future, but currently it does not seem feasible without undue effort.

@jeremystretch jeremystretch closed this as not planned Won't fix, can't repro, duplicate, stale Feb 23, 2024
@jeremystretch jeremystretch removed pending closure Requires immediate attention to avoid being closed for inactivity status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Feb 23, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature Introduction of new functionality to the application
Projects
None yet
Development

No branches or pull requests

3 participants