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

Change recently added metric key name. (very simple change) #3265

Closed
wants to merge 1 commit into from
Closed

Change recently added metric key name. (very simple change) #3265

wants to merge 1 commit into from

Conversation

upgle
Copy link
Member

@upgle upgle commented Feb 9, 2018

This PR is related to #2968(merged 2days ago) and thank you for your commit @mhenke1

I think it is more common to use dotted notation format(.) for recording metric.

Most time series db like Graphite, OpenTSDB and InfluxDB normally use a dot-delimited notation. (e.g. sys.cpu.user, invoker_docker.rm_start)

Even in the openwhisk code, metric keys for recording docker cmd use dot delimiter.

def INVOKER_RUNC_CMD(cmd: String) = LogMarkerToken(invoker, s"runc.$cmd", start)

If you are using kamon-statsd with graphite, a query is a better way.
For example, a query like invoker_containerStart.prewarmed.* will fill the variable with all possible values that exist in the wildcard position

Before

invoker_container_start_prewarmed_guest_hello_count

if action name is foo_bar and namespace is baz_qux then this marker will look ambiguous. invoker_container_start_prewarmed_baz_qux_foo_bar_count

After

invoker_containerStart.prewarmed.guest.hello_count

but in this case, dotted format is not ambiguous and can be seperated by dot(.) delimiter in Grafana, invoker_containerStart.prewarmed.baz_qux.foo_bar_count

with grafana, It is possible to query using wildcard(*) easily invoker_containerStart.prewarmed.*

image

And I also changed naming convention,
because most of the metric key names already used in the code is using camelcase like activationRun, invokerOffline and blockingActivation,

@markusthoemmes
Copy link
Contributor

@mhenke1 would you please review?

@markusthoemmes markusthoemmes self-assigned this Feb 19, 2018
@upgle
Copy link
Member Author

upgle commented Feb 24, 2018

I will close this PR because these metrics removed from #3284 PR.

@upgle upgle closed this Feb 24, 2018
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

Successfully merging this pull request may close these issues.

None yet

2 participants