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

Retrieve Topic contents with custom option - partitionid, number of last offsets #1984

Merged
merged 16 commits into from
Nov 23, 2023

Conversation

muralibasani
Copy link
Contributor

@muralibasani muralibasani commented Nov 14, 2023

Linked issue

Resolves: #1812

What kind of change does this PR introduce?

  • Bug fix
  • [ X] New feature
  • Refactor
  • Docs update
  • CI update

What is the current behavior?

Describe the state of the application before this PR. Illustrations appreciated (videos, gifs, screenshots).

On the Topic Overview under 'Topic Contents' kafka messages can be viewed.
Users have to only select from dropdown to view events, which is 5, 10, 50

What is the new behavior?

Describe the state of the application after this PR. Illustrations appreciated (videos, gifs, screenshots).
With this custom option, users can type in a partitionid and number of offsets to view

Other information

Additional changes, explanations of the approach taken, unresolved issues, necessary follow ups, etc.

Requirements (all must be checked before review)

  • The pull request title follows our guidelines
  • Tests for the changes have been added (if relevant)
  • The latest changes from the main branch have been pulled
  • pnpm lint has been run successfully

muralibasani added 2 commits November 14, 2023 17:03
Signed-off-by: muralibasani <[email protected]>
@muralibasani
Copy link
Contributor Author

image

Signed-off-by: muralibasani <[email protected]>
@muralibasani muralibasani changed the title Offset message Retrieve Topic contents with custom option - partitionid, number of last offsets Nov 15, 2023
String readMessagesType,
String clusterIdentification) {
log.info(
"readEvents bootStrapServers {}, protocol {}, clusterName {}, consumerGroupId {}, topicName {},\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

should we change this to debug before we merge?

Set<TopicPartition> topicPartitionsSet = consumer.assignment();

Set<TopicPartition> partitionsAssignment = new HashSet<>();
if (offsetPosition.equals("custom")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should make custom a constant

@@ -252,11 +252,20 @@ public Map<String, String> getTopicEvents(
String clusterIdentification,
String topic,
String offsetId,
Integer selectedPartitionId,
Integer selectedNumberOfOffsets,
String consumerGroupId,
int tenantId)
throws KlawException {
log.info(
Copy link
Contributor

Choose a reason for hiding this comment

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

lets switch this to debug before we merge as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed this debug.

consumerGroupId,
topic,
offsetId,
"partitionId",
Copy link
Contributor

Choose a reason for hiding this comment

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

partitionId & selectedNumberOfOffsets can be constants

Map<String, String> topicEvents = new TreeMap<>();
int tenantId = commonUtilsService.getTenantId(getUserName());
try {
KwClusters kwClusters =
manageDatabase
.getClusters(KafkaClustersType.KAFKA, tenantId)
.get(getEnvDetails(envId).getClusterId());
if (offsetId != null && offsetId.equals("custom")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

custom can re-use the constant

@mathieu-anderson
Copy link
Contributor

Adding the coral UI will follow this issue: #1989

The PR for this UI will be opened against this branch, and then we can merge this PR ^^

Mathieu Anderson and others added 8 commits November 17, 2023 15:22
…ion ID and custom offset (#2006)

* feat(coral): Add accessible text color for captions
* feat(coral): parse getTopicEvents params correctly
* feat(coral): Add custom offset filters
* fix(coral): Use sensible value for partition ID placeholder
* fix(coral): Add required prop to required fields
* feat(coral): Add tests
* fix(coral): Do not set grey-70 on tooltip text
* fix(coral): Make max offset 100
* fix(coral): Add badge case to a11y label
* fix(coral): Simplify legend/label color rule

---------

Signed-off-by: Mathieu Anderson <[email protected]>
Signed-off-by: Mathieu Anderson <[email protected]>
Signed-off-by: muralibasani <[email protected]>
Signed-off-by: Mathieu Anderson <[email protected]>
do {
ConsumerRecords<String, String> consumerRecords = consumer.poll(Duration.ofMillis(500));
consumerRecords.forEach(record -> eventMap.put(record.offset(), record.value()));
i++;
} while (i != numOfEventsToRead);
} while (i != numberOfPolls);
Copy link
Contributor

Choose a reason for hiding this comment

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

Id make number of polls a class constant

Mathieu Anderson and others added 3 commits November 23, 2023 11:46
Copy link
Contributor

@aindriu-aiven aindriu-aiven left a comment

Choose a reason for hiding this comment

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

LGTM

@muralibasani muralibasani merged commit 5db69c8 into main Nov 23, 2023
@muralibasani muralibasani deleted the offset-message branch November 23, 2023 11:39
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.

option to view user's choice of offset message
3 participants