Overhaul the custom fields model #3879

Closed
opened 2025-12-29 18:31:44 +01:00 by adam · 8 comments
Owner

Originally created by @jeremystretch on GitHub (Jul 22, 2020).

Originally assigned to: @jeremystretch on GitHub.

Proposed Changes

The current approach to storing custom fields employs a CustomField model for each field created by a user, and one instance of a CustomFieldValue per object per field to hold assigned values. This issue proposes replacing the current implementation with a more robust, better-performing solution yet to be identified.

One approach is to employ a new custom_fields JSONField on each model which support custom fields. This field would hold a dictionary mapping field names to their native values. This solution would retain the current CustomField model (possibly with some minor modifications) but ditch CustomFieldValue. There are several benefits to this approach:

  • Custom field data is stored locally with every instance.
  • JSON data can be filtered against directly when forming a QuerySet (e.g. Site.objects.filter(custom_fields__foo='abc')).
  • Custom field values can be set directly on an instance as dictionary keys on its custom_fields attribute.
  • Data serialization is handled transparently.

Django does not currently support writing JSON data directly via a QuerySet, however issue #29112 has been opened to implement this functionality and appears to be gaining traction. It is also something we could potentially implement locally within NetBox prior to its official implementation.

It's also worth noting that with this approach we lose the ability to easily find all values for a custom field across all models. (This can currently be achieved with e.g. CustomFieldValue.objects.filter(field=some_field).) However, I cannot think of a scenario where we've done this or where it would be especially useful.

Further investigation into this proposal is needed.

Justification

The current implementation has several drawbacks:

  • All custom field values are stored in a single database table.
  • The low-level process for assigning a new custom field value is cumbersome and unintuitive. (A new CustomFieldValue instance must be created and associated with the instance.)
  • Custom field values are serialized to string forms for storage, which can be error-prone and complicates ordering and validation.
Originally created by @jeremystretch on GitHub (Jul 22, 2020). Originally assigned to: @jeremystretch on GitHub. ### Proposed Changes The current approach to storing custom fields employs a CustomField model for each field created by a user, and one instance of a CustomFieldValue per object per field to hold assigned values. This issue proposes replacing the current implementation with a more robust, better-performing solution yet to be identified. One approach is to employ a new `custom_fields` JSONField on each model which support custom fields. This field would hold a dictionary mapping field names to their native values. This solution would retain the current CustomField model (possibly with some minor modifications) but ditch CustomFieldValue. There are several benefits to this approach: * Custom field data is stored locally with every instance. * JSON data can be filtered against directly when forming a QuerySet (e.g. `Site.objects.filter(custom_fields__foo='abc')`). * Custom field values can be set directly on an instance as dictionary keys on its `custom_fields` attribute. * Data serialization is handled transparently. Django does not currently support writing JSON data directly via a QuerySet, however [issue #29112](https://code.djangoproject.com/ticket/29112) has been opened to implement this functionality and appears to be gaining traction. It is also something we could potentially implement locally within NetBox prior to its official implementation. It's also worth noting that with this approach we lose the ability to easily find _all_ values for a custom field across all models. (This can currently be achieved with e.g. `CustomFieldValue.objects.filter(field=some_field)`.) However, I cannot think of a scenario where we've done this or where it would be especially useful. Further investigation into this proposal is needed. ### Justification The current implementation has several drawbacks: * All custom field values are stored in a single database table. * The low-level process for assigning a new custom field value is cumbersome and unintuitive. (A new CustomFieldValue instance must be created and associated with the instance.) * Custom field values are serialized to string forms for storage, which can be error-prone and complicates ordering and validation.
adam added the status: acceptedtype: housekeeping labels 2025-12-29 18:31:44 +01:00
adam closed this issue 2025-12-29 18:31:44 +01:00
Author
Owner

@paravoid commented on GitHub (Jul 24, 2020):

For the little that's worth, I think that makes a ton of sense -- I proposed it back in https://github.com/netbox-community/netbox/issues/3083#issuecomment-485546476, which was declined out of performance considerations, which I think this issue has the potential to address. I had played around with a PoC back then and it seemed feasible and scalable. I can't find the code that I wrote back then, but I doubt it'd be useful after the custom field manager changes anyway…

@paravoid commented on GitHub (Jul 24, 2020): For the little that's worth, I think that makes a ton of sense -- I proposed it back in https://github.com/netbox-community/netbox/issues/3083#issuecomment-485546476, which was declined out of performance considerations, which I think this issue has the potential to address. I had played around with a PoC back then and it seemed feasible and scalable. I can't find the code that I wrote back then, but I doubt it'd be useful after the custom field manager changes anyway…
Author
Owner

@shane-davidson commented on GitHub (Jul 27, 2020):

Another potential idea for custom field types is the ability to use existing model types as the value.

Examples to be able to create the following custom fields:
Site -> "Primary DNS" -> Links to IP
Site -> "Secondary DNS" -> Links to IP
Tenant -> "Public IP" -> Links to IP
Virtual Machine -> "Failover/DR cluster" -> Links to cluster
Linking virtual machines together - primary/secondary DB etc
Adding a secret to an IP address containing the certificate for it

Although i'm not sure how easy it would be to create/maintain the relations

@shane-davidson commented on GitHub (Jul 27, 2020): Another potential idea for custom field types is the ability to use existing model types as the value. Examples to be able to create the following custom fields: Site -> "Primary DNS" -> Links to IP Site -> "Secondary DNS" -> Links to IP Tenant -> "Public IP" -> Links to IP Virtual Machine -> "Failover/DR cluster" -> Links to cluster Linking virtual machines together - primary/secondary DB etc Adding a secret to an IP address containing the certificate for it Although i'm not sure how easy it would be to create/maintain the relations
Author
Owner

@jeremystretch commented on GitHub (Sep 16, 2020):

I've made great progress on this, however it has drawn attention to a question that I don't think we've ever really considered: What should happen when a custom field is deleted?

Current behavior (v2.9)

  • CustomField instance is deleted
  • All CustomFieldValues related to that CustomField are deleted
  • No changes are recorded against NetBox objects to which those CustomFieldValues were assigned
  • When such an object is next updated, the resulting ObjectChange record will show both the user-triggered change(s) made and the apparent deletion of the custom field data.

Obviously, the delay in reflecting the change is misleading and undesirable. I see a few options for handling custom field deletions under the new approach.

Option A: Prevent the deletion of a CustomField which still has data assigned to one or more objects. This would help mitigate accidental deletions, but also impedes intentional deletion, as it requires that all values be deleted before the field can be removed.

Option B: Allow the deletion of a CustomField but retain all data. This is no doubt the simplest approach, however it results in the retention of data which is likely no longer wanted. Also, if a custom field is deleted and a new one is later created with the same name, instances will immediately report values assigned when the original field was present.

Option C: Delete the custom field and all its values, but do not record change records. This essentially replicates NetBox's current behavior.

Option D: Delete the custom field and all its values, and record a change record for every affected object. IMO this is the most desirable action, however it should be noted that deleting a custom field may introduce a huge number of change records. This may or may not be acceptable given that the deletion of a CustomField is expected to happen very infrequently.

@jeremystretch commented on GitHub (Sep 16, 2020): I've made great progress on this, however it has drawn attention to a question that I don't think we've ever really considered: What should happen when a custom field is deleted? ### Current behavior (v2.9) * CustomField instance is deleted * All CustomFieldValues related to that CustomField are deleted * No changes are recorded against NetBox objects to which those CustomFieldValues were assigned * When such an object is next updated, the resulting ObjectChange record will show both the user-triggered change(s) made _and_ the apparent deletion of the custom field data. Obviously, the delay in reflecting the change is misleading and undesirable. I see a few options for handling custom field deletions under the new approach. **Option A:** Prevent the deletion of a CustomField which still has data assigned to one or more objects. This would help mitigate accidental deletions, but also impedes intentional deletion, as it requires that all values be deleted before the field can be removed. **Option B:** Allow the deletion of a CustomField but retain all data. This is no doubt the simplest approach, however it results in the retention of data which is likely no longer wanted. Also, if a custom field is deleted and a new one is later created with the same name, instances will immediately report values assigned when the original field was present. **Option C:** Delete the custom field and all its values, but do not record change records. This essentially replicates NetBox's current behavior. **Option D:** Delete the custom field and all its values, _and_ record a change record for every affected object. IMO this is the most desirable action, however it should be noted that deleting a custom field may introduce a huge number of change records. This may or may not be acceptable given that the deletion of a CustomField is expected to happen very infrequently.
Author
Owner

@dgarros commented on GitHub (Sep 16, 2020):

I think D would be ideal and as mentioned it shouldn't happen too often.

@dgarros commented on GitHub (Sep 16, 2020): I think D would be ideal and as mentioned it shouldn't happen too often.
Author
Owner

@itdependsnetworks commented on GitHub (Sep 16, 2020):

Agreed on option D

@itdependsnetworks commented on GitHub (Sep 16, 2020): Agreed on option D
Author
Owner

@jeremystretch commented on GitHub (Sep 16, 2020):

I've got a proof-of-concept working for the stale data cleanup in 2d56a658, using the m2m_changed and pre_delete signals to identify the affected object types. Seems like a reasonable approach, though further testing is needed.

@jeremystretch commented on GitHub (Sep 16, 2020): I've got a proof-of-concept working for the stale data cleanup in 2d56a658, using the `m2m_changed` and `pre_delete` signals to identify the affected object types. Seems like a reasonable approach, though further testing is needed.
Author
Owner

@jeremystretch commented on GitHub (Sep 17, 2020):

I've merged this work int odevelop-2.10. Although further work and testing may be needed, the implementation itself can be considered complete.

@jeremystretch commented on GitHub (Sep 17, 2020): I've merged this work int o`develop-2.10`. Although further work and testing may be needed, the implementation itself can be considered complete.
Author
Owner

@paravoid commented on GitHub (Sep 17, 2020):

That's so awesome to see, thanks so much @jeremystretch! 👍

One (hopefully quick) question: I noticed that you've added custom fields to all primary models, and said in 5146 that it's explicitly not for other models at this time. Would it be possible to elaborate on what's missing in terms of functionality/limitations in order for this to be added to other models? Asking so that future contributors (possibly including myself ;)) know what to expect in terms of difficulty :) (The one I care about FWIW at the moment is InventoryItem, to add some tracking fields to items like Juniper routing engines and linecards).

@paravoid commented on GitHub (Sep 17, 2020): That's so awesome to see, thanks so much @jeremystretch! :+1: One (hopefully quick) question: I noticed that you've added custom fields to all primary models, and said in 5146 that it's explicitly not for other models at this time. Would it be possible to elaborate on what's missing in terms of functionality/limitations in order for this to be added to other models? Asking so that future contributors (possibly including myself ;)) know what to expect in terms of difficulty :) (The one I care about FWIW at the moment is `InventoryItem`, to add some tracking fields to items like Juniper routing engines and linecards).
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#3879