Graylog2
Units support - BE
Description
Hardcoded list of supported units. Fields can have unit associated with them. Some basic GL fields get default/hardcoded unit association. /nocl
Fixes #19157 Fixes #19299
Motivation and Context
- Do not hardcore units in FE.
- Encode conversion of units inside them.
- Group units by types.
- Use units for some fields without a need to configure anything in the widget.
How Has This Been Tested?
Manually and with new unit tests.
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Refactoring (non-breaking change)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
Checklist:
- [x] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have read the CONTRIBUTING document.
- [x] I have added tests to cover my changes.
What's the use case for storing the units in the database? There aren't that many units for data values. Why not just hardcode them in the UI code?
What's the use case for storing the units in the database? There aren't that many units for data values. Why not just hardcode them in the UI code?
We would like to avoid the necessity to change the code if a new unit is needed. We hope it can be just added to DB and the rest will work transparently.
We also consider storing field-unit relation, especially for GL own fields, but probably for all other fields as well.
We would like to avoid the necessity to change the code if a new unit is needed. We hope it can be just added to DB and the rest will work transparently.
Hmm, can't we check which units would be needed and ship a pre-defined list? There aren't that many different common units. That would avoid the database collection, API endpoint, and the code for all of that.
We also consider storing field-unit relation, especially for GL own fields, but probably for all other fields as well.
Okay, got it. How about pre-define the units (enum?) and then use it for the field-unit relation. For the UI we could use our existing infrastructure to generate TypeScript files for the pre-defined units so can maintain them in a single place.
What do you think?
Hmm, can't we check which units would be needed and ship a pre-defined list?
We can, but then we end up with a close list of units. With this approach if someone needs a new set of units, it is easy to expand and upgrade. We can add migrations to add new unit sets, throw some new units manually into DB if rich client pays for this, or even create a UI for importing/creating additional unit at some point in the future, if needed.
There aren't that many different common units. That would avoid the database collection, API endpoint, and the code for all of that.
Is it really problematic that we have a new collection? It won't be big, so I assume it is not storage that you are worried about, but the existence of the collection itself? When it comes to the code, probably a similar amount of code would be needed in the FE if we moved it there (now Resource class is tiny, so is Service if you exclude derived units generation for basic prefixes (giga, mega, nano etc.), which would need to be repeated, or hardcoded in FE anyway with your approach ).
Okay, got it. How about pre-define the units (enum?) and then use it for the field-unit relation. For the UI we could use our existing infrastructure to generate TypeScript files for the pre-defined units so can maintain them in a single place.
You mean that somewhere in the DB we would refer to FE-only enum values?
@bernd - FYI, we are supposed to provide around 13 units initially. Imho it is quite likely new units may be requested by customers (length, imperial length, temperature, transfer(in bits?), weight), so if they liked the feature and if we have indeed a variety of use-case scenarios/users, the list could easily cover 50-100+ units. My attempt was to be ready for extension without significant difficulties.
I doubt that we need more than this: https://github.com/grafana/grafana/blob/2d9d0e61b132b3eecc7a9c61a72235dbd19d9711/packages%2Fgrafana-data%2Fsrc%2FvalueFormats%2Fcategories.ts#L37-L439
😄
I doubt that we need more than this: https://github.com/grafana/grafana/blob/2d9d0e61b132b3eecc7a9c61a72235dbd19d9711/packages%2Fgrafana-data%2Fsrc%2FvalueFormats%2Fcategories.ts#L37-L439
😄
Heh, it seems they don't even support auto-scaling yet, at least not in this file. ;)
@bernd - in order to proceed with the topic, I could also introduce a second implementation of the unit service, in memory one, and work on that in the near future. It would be faster than our current discussion and we will be able to switch to the db service if I managed to persuade you that we need it at some point.
What do you think? :)
Sorry for the sluggish responses. I was at an off-site meeting.
Is it really problematic that we have a new collection? It won't be big, so I assume it is not storage that you are worried about, but the existence of the collection itself? When it comes to the code, probably a similar amount of code would be needed in the FE if we moved it there (now Resource class is tiny, so is Service if you exclude derived units generation for basic prefixes (giga, mega, nano etc.), which would need to be repeated, or hardcoded in FE anyway with your approach ).
The additional database collection is not the issue. To me, making a finite and well-known collection of units configurable via the database and having an API for them introduces unneeded complexity.
I think there is a limited number of units that make sense in a Graylog context. It's also easy to add more in a new release when customers request them. (also easier to backport when we only have to maintain a static list) Until now we don't have any units, so it's probably okay for customers to wait for the next release to get more exotic ones. :smile:
You mean that somewhere in the DB we would refer to FE-only enum values?
What I had in mind was to define the units in a resource file. For the frontend, we could generate TypeScript code from that resource file at build time. That way, we don't need an API endpoint and HTTP requests to get the available units. Similar to how we generate TypeScript code for HTTP API models.
@bernd - in order to proceed with the topic, I could also introduce a second implementation of the unit service, in memory one, and work on that in the near future. It would be faster than our current discussion and we will be able to switch to the db service if I managed to persuade you that we need it at some point.
If you and your team think we need the ability to make units configurable via the database and it's worth the added complexity, you should continue with your current approach.
@luk-kaminski If you continue with the database approach, please remember to implement content-pack support for units and add dependency resolution for the view entities. Otherwise, the referenced units in views might be broken.
@luk-kaminski If you continue with the database approach, please remember to implement content-pack support for units and add dependency resolution for the view entities. Otherwise, the referenced units in views might be broken.
We are not sure about the decision yet. Will hopefully have a meeting tomorrow.
@luk-kaminski, @bernd: I would say we should move these definitions to the code. Previously, I was torn between the complexity of the implementation and the fact that it is already implemented, but the fact that we would still need to add content pack support pushes me over the edge towards a simpler solution.
