feat(clients/go): add metrics for job worker

[AndreasBieber]

Description

Introduces an interface JobWorkerMetrics which can be registered with the JobWorkerBuilder:

builder.Metrics(metricsImpl)

Related issues

closes #4500

Pull Request Checklist

• All commit messages match our commit message guidelines
• The submitting code follows our code style
• If submitting code, please run mvn clean install -DskipTests locally before committing (no java code changes)

[CLAassistant]

All committers have signed the CLA.

[npepinpe]

Thanks a lot for this PR and your patience 🙂

LGTM, can you also add a test for the new builder method in jobWorker_builder_test.go?

One small question, I'll admit I'm not a Go expert - as you can see most of our project is in Java. This is the first time we add metrics in the Go client; on the Java side, I'd usually recommend using something like Micrometer as a metrics facade - is there something similar in the Go ecosystem?

[AndreasBieber]

We usually use the Prometheus client to expose metrics, as this is the de facto standard in the cloud native environment (github.com/prometheus/client_golang).

But I did not want to introduce a new dependency in the zeebe go client.

[AndreasBieber]

Actually I wanted to pass a metric facade to the client. However, this would have resulted in breaking changes, because the abstraction is not sufficiently separated. This would have had the advantage to capture much more metrics. Perhaps this can be addressed in the future.

[npepinpe]

We usually use the Prometheus client to expose metrics, as this is the de facto standard in the cloud native environment (github.com/prometheus/client_golang).

But I did not want to introduce a new dependency in the zeebe go client.

Totally agree - we made that (imo) mistake in our main project but I would love to replace that with Micrometer.

Actually I wanted to pass a metric facade to the client. However, this would have resulted in breaking changes, because the abstraction is not sufficiently separated. This would have had the advantage to capture much more metrics. Perhaps this can be addressed in the future.

I'd be happy to discuss what you have in mind there 🙂 I think the changes you added are fairly easy to deprecate in favor of a facade in the future, so I don't see any problem there

EDIT: Regarding the PR, it's basically approved but I want to try it out concretely with a Prometheus client, so give me a few minutes

[npepinpe]

Tested it with Prometheus, looks good 👍

[npepinpe]

One small thing, sorry I didn't flag it before; we keep track of backwards compatibility, and this breaks it (you can read more here). Now that's partially because we're Java devs and using interfaces in Go in the wrong way (for which we have an issue, see here).

In my opinion, this change is worth breaking, as the benefits outweigh the gains, and I doubt many users are implementing the job worker builder interface.

If you could then run

gocompat save ./...

And push that; you can verify it now passes the compat check by running

gocompat compare --go1compat ./...

Thanks! Sorry for the back and forth 😅

[npepinpe]

Thanks so much, and thanks for being patient with us 🙂

This will be part of the next release, 0.24, scheduled for the first week of July, though there weren't much changes since 0.23 in the Go client so you could probably use (at your own risk) the commit tag.

[npepinpe]

bors r+

[zeebe-bors]

Build failed

• continuous-integration/jenkins/branch

[npepinpe]

bors retry

[zeebe-bors]

Build failed

• continuous-integration/jenkins/branch

[npepinpe]

bors r+

[zeebe-bors]

Build failed

• continuous-integration/jenkins/branch

[npepinpe]

Sorry, pesky flaky tests 😅

bors retry

[zeebe-bors]

Build succeeded

• continuous-integration/jenkins/branch