Add support to create "metric" as a istio resource

[anmolbabu]

When I try to create a metric as in https://github.com/anmolbabu/hdd-istio-java-poc, it fails with the following error:

java.lang.IllegalArgumentException: metric is not a known Istio resource. at me.snowdrop.istio.client.IstioClient.registerCustomResources(IstioClient.java:50) at me.snowdrop.istio.client.IstioClient.registerCustomResources(IstioClient.java:72) at org.osio.hdd.istio.com.App.main(App.java:44)

On some debugging, I found that it was failing at: https://github.com/snowdrop/istio-java-api/blob/master/istio-common/src/main/java/me/snowdrop/istio/api/internal/IstioSpecRegistry.java#L78 as it seems from https://github.com/snowdrop/istio-java-api/blob/master/istio-common/src/main/java/me/snowdrop/istio/api/internal/IstioSpecRegistry.java#L45-L47 that the type "metric" is not supported.

Note: The metric I am trying to create is the same as in the istio tutorial @ https://github.com/redhat-developer-demos/istio-tutorial#custom-metrics

[metacosm]

Indeed, this isn't currently supported but I will add support for it as soon as possible. Thanks for the report!

[bgokden]

@anmolbabu We had a similar issue we end up adding our MixerMetric Structure and Prometheus Structure.

https://github.com/magneticio/istio-java-api/tree/master/istio-model/src/main/java/me/snowdrop/istio/api/model/v1/mixer/template

We didn't test it well enough. If it passes our tests, we will create a pull request.

[metacosm]

@bgokden Thank you! This project is still in its infancy and figuring out which resources need to be generated is not as straightforward as it should be… but we'll get there in time! :)

[metacosm]

I have started working on this. I hope to have a fix by the end of the week. By the way, @anmolbabu, you should definitely use the latest version: we're up to 0.9, you're still using 0.7. I need to figure out what versioning scheme to adopt since it'd be nice that the lib match the Istio version it is compatible with.

[metacosm]

Metrics should work now. I still need to work on the other CRDs, though.

[anmolbabu]

Could you please also add "kind: prometheus" as in https://github.com/redhat-developer-demos/istio-tutorial/blob/master/istiofiles/recommendation_requestcount.yml#L16 and "kind: rule" as in https://github.com/redhat-developer-demos/istio-tutorial/blob/master/istiofiles/recommendation_requestcount.yml#L32

[metacosm]

Yes, it’s planned

Le 10 mai 2018 à 13:04, anmolbabu notifications@github.com a écrit :

Could you please also add "kind: prometheus" as in https://github.com/redhat-developer-demos/istio-tutorial/blob/master/istiofiles/recommendation_requestcount.yml#L16 and "kind: rule" as in https://github.com/redhat-developer-demos/istio-tutorial/blob/master/istiofiles/recommendation_requestcount.yml#L32

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

[metacosm]

BTW, @anmolbabu are these files confirmed to work on Istio 0.7.1 because the documentation (https://istio.io/docs/reference/config/adapters/prometheus.html) states that, for example, https://github.com/redhat-developer-demos/istio-tutorial/blob/master/istiofiles/recommendation_requestcount.yml#L23 should be instanceName and not instance_name?

[anmolbabu]

@metacosm Yes I tried https://istio.io/docs/tasks/telemetry/metrics-logs.html#collecting-new-telemetry-data on istio-0.7.1 and instance_name still works

[metacosm]

@anmolbabu actually, based on what I've learned since I asked the question, it works because the Istio parser is lenient and accepts both versions. However, field names are supposed to use lowerCamelCase. Supporting both in the Java API, while possible, would make things more complex so right now, I tend to think that I will only support the lowerCamelCase format (i.e. instanceName). What do you think?

[anmolbabu]

@metacosm I'll give this("_" notation/snake case replaced with camel case) a try on my deployment and confirm max by tomorrow EOD

[anmolbabu]

@metacosm You are right, the camel case works

[metacosm]

@anmolbabu thanks for checking!

[metacosm]

This has been fixed with the 0.10 version released yesterday.