Skip to content
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

Named tags and additional tags #56

Open
mwringe opened this issue Dec 5, 2016 · 8 comments
Open

Named tags and additional tags #56

mwringe opened this issue Dec 5, 2016 · 8 comments
Assignees

Comments

@mwringe
Copy link
Contributor

mwringe commented Dec 5, 2016

All metadata about a metric in Hawkular Metrics is stored as a tag. Right now when someone creates a metric definition, they are able to add to that metric additional tags they would like to add. But there may be special tags we want to apply to metrics in a more structured manner.

For instance, having a 'unit' tag would be very useful to determine what unit the metric is being gathered in.

Currently a user could add a 'unit' tag to the list of tags, and this would work, but since its a part of a custom list its hard to enforce this naming. Some people may call it 'unit' while others may call it 'units', or they may have a mix themselves between the pods they are monitoring.

It may make sense to move something like this outside of the 'tags' list and to rename the 'tags' list to 'additional-tags' (which also makes it clear that this is not the only list of tags to be applied to the metric, but a list of extra ones to be added).

Anything outside of the 'additonal-tags' list would have to be properly named so there would be no issues with expected tags being under a slightly different tag name.

eg:

endpoints:
- type: jolokia
  metrics:
  - name: java.lang.type=Threading#ThreadCount
    type: gauge
    unit: threads
    additional-tags:
      my-metric-tag: the-metric-tag-value
      my-other-metric-tag: the-metric-tag-value
@pilhuhn
Copy link
Member

pilhuhn commented Dec 6, 2016

I think we should pull out the unit in a separate field (which may be implemented as tag), but which lives next to id, type and tags etc. on the MetricDefinition. On top of that we need to follow one of the "standards" like metric2.0 or JSR 363 (UoM) or even just what we had in the past in RHQ at https://github.com/rhq-project/rhq/blob/master/modules/core/domain/src/main/java/org/rhq/core/domain/measurement/MeasurementUnits.java

@mwringe
Copy link
Contributor Author

mwringe commented Dec 6, 2016

I think we should pull out the unit in a separate field (which may be implemented as tag), but which lives next to id, type and tags etc. on the MetricDefinition.

Yep, which is exactly what my example above does. Its a separate field and it gets stored as a tag (since that is the only option in Hawkular Metrcs) and we rename the old 'tag' to 'additional-tags' to make it more clear its something separate from what they system will provide. We could even call it 'custom-tags' if that makes it more clear.

On top of that we need to follow one of the "standards" like metric2.0 or JSR 363 (UoM)

hmm, I don't know how this is going to work exactly. The list of units is potentially infinite and not known before hand.

If this was json and not yaml, I could see having something like unit: kb to represent a known unit type and something like unit: "bananas" to represent a custom unit type. So adding quotes means its a custom string unit and not a known type, but we don't have that option in yaml.

Any idea on how this should work nicely? Maybe require a special way of specifying it? eg unit: custom.bananas, custom-unit: bananas, unit: #bananas, etc

@pilhuhn
Copy link
Member

pilhuhn commented Dec 6, 2016 via email

@mwringe
Copy link
Contributor Author

mwringe commented Dec 6, 2016

So those would be merged down into the normal tags
in a MD on write to Hawkular_metrics? And for reading-back
you would pull out the "known" tags like unit and move
everything else under additional-tags?

Essentially its just a more clear way of defining tags and allows us to check certain fields better. If its not in the additional tags section it needs to be named properly and (potentially) be of a known list of values. How it would get written in Hawkular Metrics is exactly the same.

No sure. For me a metric would either be without a unit.
Then it is "pieces" (or number of things).
Otherwise it will be some think like kb, kB, km etc.

Things like kb, km, etc are also just "pieces" :) And it can also be "pieces per some_other_metric" (eg time, distance, etc). And potentially in another form that I cannot think of right now.

I definitely think we need to have known values here (especially since if I use something like 'bytes' and its a really large number, it may make sense to display as 'kb' or 'gb'. Same with time when it stored as ns but it should really be graphed as minutes or hours).

I just don't know the best way to specify if its a known unit or a custom one.

I would here set a unit of 'none' and if needed add some
soft of description "banana"

I don't know if specifying 'unit' as none and then adding an additional value to store the custom value is the best solution. It might be easier if we had just one parameter that needs to be specified instead of having to specify two in this case.

@jmazzitelli
Copy link
Contributor

jmazzitelli commented Dec 6, 2016

We would just add a "units" attribute like @mwringe says above. Its value can be any string (so can support custom units).

H-Metrics doesn't support "units" as a first-class concept in the metric definitions - http://www.hawkular.org/docs/rest/rest-metrics.html#Metric - so these units can be anything the user wants and we will add it to the metric definition as a tag named "units".

As I understand the concerns above, the issue is we want to make sure the user uses specific abbreviations for specific well-known units (e.g. we want users to use "ms" when they mean milliseconds - we do not want them using "msecs" or "millis"). If they want to define a custom units identifier, we will ask them to prefix a custom metric with "custom:" - any unit string that is not prefixed with "custom:" will be assumed to be a standard unit and must match one of the well known units. If the string then doesn't match a fixed unit, we abort, log a warning, or something like that. If it starts with "custom:" we strip that off and use the resultant string as the custom unit (i.e. "custom:foobar" becomes the units of "foobar")

So, for example, suppose we want to support these fixed, common units of "b", "kb", "mb", "gb", "tb", "s", and "ms" (obviously, we'll have a lot more, this is just an example):

https://play.golang.org/p/1XlTLjF2Mf

package main

import (
	"fmt"
	"strings"
)

const CustomUnitPrefix = "custom:"
type UnitsType string
type StandardUnitsType []UnitsType
var StandardUnits = StandardUnitsType{"b", "kb", "mb", "gb", "tb", "s", "ms"}

// Checks if the given string is a valid units identifier and if it is, returns it as a UnitsType.
// If it is not a valid units identifier, an err is returned.
// If it is a custom units identifier, it is returned minus the custom unit prefix.
func (*StandardUnitsType) GetUnits(u string) (UnitsType, error) {
	if len(u) > len(CustomUnitPrefix) && strings.HasPrefix(u, CustomUnitPrefix) {
		return UnitsType(strings.TrimPrefix(u, CustomUnitPrefix)), nil
	}
	for _, x := range StandardUnits {
		if string(x) == u {
			return x, nil
		}
	}
	return "", fmt.Errorf("invalid units: " + u)
}

func main() {
	u, e := StandardUnits.GetUnits("ms")
	fmt.Println("'ms'             is a valid unit     -->", u, "; error=", e)
	u, e = StandardUnits.GetUnits("millis")
	fmt.Println("'millis'         is NOT a valid unit -->", u, "; error=", e)
	u, e = StandardUnits.GetUnits("custom:foobars")
	fmt.Println("'custom:foobars' is a valid unit     -->", u, "; error=", e)
	u, e = StandardUnits.GetUnits("foobars")
	fmt.Println("'foobars'        is NOT a valid unit -->", u, "; error=", e)
}

returns:

'ms'             is a valid unit     --> ms ; error= <nil>
'millis'         is NOT a valid unit -->  ; error= invalid units: millis
'custom:foobars' is a valid unit     --> foobars ; error= <nil>
'foobars'        is NOT a valid unit -->  ; error= invalid units: foobars

@jmazzitelli
Copy link
Contributor

I'm going to try to refactor the code to support additional-tags and "fixed" tags (like "units" and anything other that we come up with). We can discuss further more about the details of "units" - but I think what has been discussed earlier is good. We have only one attribute to ask the user to define ("units") and if it is a custom one, we just expect them to prefix it with something (like "custom:"), otherwise, the value must be a specific, well-known units identifier (like "ms" or "kb" or whatever). "units" can be optional, by the way - a metric doesn't have to have a units specified (this is heiko's "none"). Prime example is "number of connections in connection pool" or "number of requests processed". But in this case, we don't have to create a units label. A missing units label implies "none".

@jmazzitelli jmazzitelli self-assigned this Dec 7, 2016
@jmazzitelli
Copy link
Contributor

see PR #58 for code that stores units as a fixed tag. If there are no units, no tag is created. There is place in the code for us to put any other fixed tags that we might want to add in the future (see metrics_collector_manager.go and the comment "// the metric tags will consist of the custom tags as well as the fixed tags"). We would build up the metricTags map with any fixed tags we want in the future. Right now, there is just "units"

@jmazzitelli
Copy link
Contributor

I merged that PR. We now have a first-class attribute "units" like we discussed above whose value is stored as a tag "units" on the H-Metrics metric definition. Also as discussed, if the value is prefixed with "custom:" then the units will be stored as is ("custom:foobar" will be stored as "foobar" in the tag "units"). If not prefixed with "custom:" then the units symbol must match one of the standard ones. I built an initial list of standard units symbols based on international standards (SI) and metrics20 - see metric_units.go

I did not yet rename "tags to "custom-tags". I figure leave things as-is unless/until it really shows itself to be a problem. I don't think it is a issue to worry about right now.

If in the future we want to add more, we just add the fixed tags to the metric definition via MetricsCollectorManager - we would add the YAML attribute to MonitoredMetric struct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants