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:
Site.objects.filter(custom_fields__foo='abc')).custom_fields attribute.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.
The current implementation has several drawbacks:
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…
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
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?
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.
I think D would be ideal and as mentioned it shouldn't happen too often.
Agreed on option D
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.
I've merged this work int odevelop-2.10. Although further work and testing may be needed, the implementation itself can be considered complete.
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).
Most helpful comment
Agreed on option D