Skip to content
This repository has been archived by the owner on Mar 17, 2024. It is now read-only.

Support consumer groups for which member information is unavailable. #128

Merged
merged 2 commits into from
May 8, 2020

Conversation

lilyevsky
Copy link
Contributor

This is a fix for the issue #126 .
It allows to report lag for the consumer groups that don't have members information.
Such situation is actually happening for the consumers running under Flink.

Also minor fix in setting properties, KafkaClient.scala lines 61 and 73:
the documentation for java.util.Properties suggests not to use putAll method because it is not type-safe (in fact, in my environment those two lines did not compile). They suggest using setProperty method.

I tested this fix in my local environment.

Also minor fix in setting properties, KafkaClient.scala lines 61 and 73:
the documentation for java.util.Properties suggests not to use putAll method because it is not type-safe (in fact, in my environment those two lines did not even compile). They suggest using setProperty method.
@lightbend-cla-validator
Copy link

At least one pull request committer is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user

@yazgoo yazgoo mentioned this pull request Mar 9, 2020
@yazgoo
Copy link
Contributor

yazgoo commented Mar 9, 2020

Hi @lilyevsky . I'm having the same issues with flink and simple consumer groups
#124 is a try at fixing that, but maybe your implementation is better, wdyt @seglo ?

@seglo
Copy link
Owner

seglo commented Mar 9, 2020

Kafka 2.4.1 will be released shortly, which fixes a different issue in Kafka Lag Exporter. I plan to cut a release then, so let's pick one of these solutions soon.

Copy link
Owner

@seglo seglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks a little more succinct than #124, but I like that #124 gives this type of consumer a name, "simple consumers". Let's use that here.

It would be nice to have a test for this. Can either of you (@yazgoo or @lilyevsky) volunteer to write one?

@lightbend-cla-validator
Copy link

At least one pull request committer is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user

@aklimko
Copy link

aklimko commented Apr 23, 2020

Any update on this? 🤔

@seglo
Copy link
Owner

seglo commented Apr 23, 2020

@lilyevsky Do you mind signing the CLA so I can merge this PR? It might be necessary to recreate the PR to retrigger the check.

@lilyevsky
Copy link
Contributor Author

@seglo I will work on the CLA. Need to do the corporate one, hopefully can be done in a couple of days.

@lilyevsky
Copy link
Contributor Author

I am waiting for CLA approval by our Legal department, that may take some time. Hopefully within a week I will have it.

@seglo
Copy link
Owner

seglo commented Apr 28, 2020

Thanks for the update. I may cut a release in the meantime, but we can push another one once your PR is merged.

seglo added a commit that referenced this pull request May 7, 2020
@seglo
Copy link
Owner

seglo commented May 7, 2020

I released 0.6.1. We can do another release when you get approval. Or if you like I could implement this myself.

@lilyevsky
Copy link
Contributor Author

@seglo I got the CLA, will upload it shortly

@lilyevsky
Copy link
Contributor Author

@seglo I just sent an email with the attached CLA, CCd to you

@lilyevsky
Copy link
Contributor Author

@seglo I confirmed the CLA with my GitHub account and I got the email response from [email protected]

@JustinPihony JustinPihony reopened this May 8, 2020
@lightbend-cla-validator
Copy link

At least one pull request committer is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user

@JustinPihony
Copy link

Hi @lilyevsky this is now failing because your github account does not have your email attached to it. You can see that by looking at the commits and how your username is not clickable to your account. Would you be able to add your email so that github can match you accordingly, and we can then re-run the CLA validator?

@lilyevsky
Copy link
Contributor Author

Hi @JustinPihony I just edited my profile, added both primary email [email protected] and my work email [email protected]. I see my username is now clickable.
Please try running CLA validator again.

@JustinPihony
Copy link

Great! You’re all set. I’ll leave it to @seglo or another maintainer to merge :) thanks!

@seglo
Copy link
Owner

seglo commented May 8, 2020

Thanks @JustinPihony

@seglo seglo merged commit b48d5ba into seglo:master May 8, 2020
@jweibel22
Copy link

Hi @seglo, I'm also waiting for this feature, what's the timeline for the next release?

@s2490
Copy link

s2490 commented Jun 1, 2020

Hi @seglo is this fix is included in 0.6.2?
Asking because we have upgraded to latest version and issue is still there 🤔

@lilyevsky
Copy link
Contributor Author

@s2490 I tested 0.6.2 this morning and it works for me. Maybe your issue is different?

@s2490
Copy link

s2490 commented Jun 2, 2020

@lilyevsky that could be the case, from CLI perspective example lags looks as follows:

TOPIC           PARTITION  CURRENT-OFFSET  LOG-END-OFFSET  LAG             CONSUMER-ID                                   HOST            CLIENT-ID
topic001        0          74893           172550          97657           client002-4c82c1e2-c868-491b-a22f-751914058fa4 /172.20.52.117  client002
topic001        1          117689          172511          54822           client002-4c82c1e2-c868-491b-a22f-751914058fa4 /172.20.52.117  client002
topic001        2          21398           172639          151241          client002-4c82c1e2-c868-491b-a22f-751914058fa4 /172.20.52.117  client002
topic002        0          49740           50964           1224            -                                             -               -
topic002        2          49898           51166           1268            -                                             -               -
topic002        1          49663           50870           1207            -                                             -               -

Please note that some client details are available, others not. For such cases exporter isn't reporting back metrics for topic002 at all, on our environments such situations are pretty common because of custom partition assignment usage.

Surprisingly when all client details are unavailable exporter is showing correct values, so my example could be some kind of corner case, wdyt?

@s2490
Copy link

s2490 commented Jun 23, 2020

@lilyevsky have you tested my case maybe or I should create dedicated issue for it?

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

Successfully merging this pull request may close these issues.

Continue to calculate lag for inactive groups for a configurable timespan
8 participants