-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Ebookbug ipmi lowercase change #919
Conversation
b31f0f7
to
98fb22d
Compare
hi @ebookbug, could you take a look at this? I have made some changes to add unit tests, to add a "units" tag, to convert everything to lowercase, and to convert spaces to "_" can you let me know what you think of doing this? I thought it might make it easier and more consistent to query in the db? |
Would also be OK with me if we only translated " " to "_", we don't need to do the lower casing if you'd rather not |
ba0ab94
to
e60072a
Compare
Tansform to lower case is ok. |
Can you explain why you would prefer units as a field instead of a tag? |
This is no need to index the unit |
@ebookbug I see your point...it certainly seems like it makes more sense as a field for influxdb. There are other outputs though where it makes more sense as a tag (graphite, opentsdb, prometheus, etc.) how would you feel if the schema design actually looked more like this (would this be too difficult to query?):
|
One other thing to consider-- if So I'm leaning towards leaving it as-is, as a tag |
Different manufacture‘s server maybe have different metrics. But in next build,we can seperate the measurement ipmi_sensor to some sub measurements ,such as ipmi_temp,ipmi_fan_speed,ipmi_voltage .... |
It's a common issue to discuss where to place the unit |
2d4e436
to
8beacf9
Compare
No description provided.