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

when mapping inventory-metric ID to hawkular-metric ID, check if metrid-id property exists #110

Closed
jmazzitelli opened this issue Jul 9, 2016 · 6 comments

Comments

@jmazzitelli
Copy link
Contributor

There is a use-case to give users some freedom and flexibility to choose what their hawkular-metric IDs look like, rather than force them to adopt the rule "inventory metric ID is always the same as hawkular metric ID".

The hawkular wildfly agent is introducing an optional "metric-id" property found on the inventory metric instance definition. When this property exists, the hawkular-metric ID is the value of that property (it is not the value of that inventory metric instance ID).

The current behavior will remain the same and will become the default - inventory metric instance ID is the same as the hawkular-metric ID - they will only be different if someone adds that "metric-id" property in inventory.

See the HWKAGENT JIRA issue and associated PR where the agent creates this metric-id when appropriate: https://issues.jboss.org/browse/HWKAGENT-78

@josejulio
Copy link
Member

AFAIK the API doesn't do/use any inventory data to query a metric endpoint. It's left to the developer to decide how to mix inventory/metric data.

We could add this warning to the Readme or we could add the attribute metric_id to Hawkular::Inventory::Metric and set it to metric-id if provided or id if not, so the developer wouldn't need to know about this.
// cc @pilhuhn

@pilhuhn
Copy link
Member

pilhuhn commented Jul 19, 2016

@josejulio Yeah, I think the latter is what @jmazzitelli meant. We always populate a field with metric-id or fall back to id.
I wonder (and this is the other side of my comment on the HWKAGENT-78 jira) if we should not just return this metric-id as id if it exists or otherwise the real id.
I see a potential conflict though for this idea: what if we ever want to update the metric definition - how would we then find the real id on which it is required from inventory. I guess we could have a getter "getId" instead of the :id accessor and make this do the magic. While the update would take the real id, that is still present on the object.

@josejulio
Copy link
Member

If is not mandatory for the developer to know the real id of the inventory_metric we could manually implement the attr :id (as getId doesn't seem very ruby-ish) to return the metric_id (if available) so this change would be even more transparent.
In the other hand, if is required for the dev to know the id, replacing it isn't an option and we could have another id (metric_id or native_metric_id, hawkular_metric_id, etc)

@jmazzitelli
Copy link
Contributor Author

I just commented the following in hwkagent-78, but I will post here too:

Heiko asked "why not directly set the id of the metric definition in inventory to the metric-id here?"

The reason is due to the implementation and how the MeasurementInstance objects are immutable. When the measurement instance IDs are generated, it happens as we build up the Resource using a builder API - this is when we create these instances with our own internal IDs. Once we build the resource, then we pass that instance to our sampling service impl to generate a metric ID and attach it to the measurement instance.

It does not have to work this way. I think we can change the implementation to do something like make the ID mutable and change it - or somehow get the builder to create the ID on the fly somehow. It was just easier implementing it the way it is now. But if this extra "metric-id" property is causing problems and we do not want to have it, we can try to fix it and make it go away.

@jmazzitelli
Copy link
Contributor Author

Did this ever get fixed? I don't see a related PR documented here.

@josejulio
Copy link
Member

josejulio commented Aug 18, 2016

Not yet, I've plans on doing it. But haven't setup my env to have a domain eap (and then i'll have to setup a custom metric_id template).
Do you have an environment or config handy ? I'm currently testing with SSl and don't want to do all over again before finishing.

josejulio added a commit to josejulio/hawkular-client-ruby that referenced this issue Sep 1, 2016
pilhuhn pushed a commit that referenced this issue Sep 15, 2016
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