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

[SNOW-870373] Enable JMX metrics for Snowpipe Streaming #674

Merged
merged 19 commits into from
Aug 25, 2023

Conversation

sfc-gh-rcheng
Copy link
Collaborator

@sfc-gh-rcheng sfc-gh-rcheng commented Jul 25, 2023

Tracks the offsetPersistedInSnowflake, processedOffset and latestConsumerOffset for Snowpipe Streaming. This PR only enables the JMX metrics, further metrics may be added in the future as needed, please comment any metrics you think would be useful to track!

image

@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #674 (5a6b603) into master (58b9247) will increase coverage by 0.06%.
The diff coverage is 91.51%.

@@            Coverage Diff             @@
##           master     #674      +/-   ##
==========================================
+ Coverage   87.95%   88.02%   +0.06%     
==========================================
  Files          50       52       +2     
  Lines        4144     4266     +122     
  Branches      451      455       +4     
==========================================
+ Hits         3645     3755     +110     
- Misses        331      342      +11     
- Partials      168      169       +1     
Files Changed Coverage Δ
.../kafka/connector/SnowflakeSinkConnectorConfig.java 88.19% <ø> (ø)
.../kafka/connector/internal/metrics/MetricsUtil.java 90.00% <ø> (ø)
...nternal/streaming/SnowflakeTelemetryServiceV2.java 100.00% <ø> (+16.66%) ⬆️
...ternal/telemetry/SnowflakeTelemetryPipeStatus.java 97.72% <ø> (ø)
...nternal/telemetry/SnowflakeTelemetryServiceV1.java 100.00% <ø> (ø)
...nnector/internal/telemetry/TelemetryConstants.java 0.00% <ø> (ø)
...nal/streaming/SnowflakeTelemetryChannelStatus.java 84.93% <84.93%> (ø)
...l/streaming/SnowflakeTelemetryChannelCreation.java 86.66% <86.66%> (ø)
...ctor/internal/streaming/TopicPartitionChannel.java 91.98% <98.36%> (+0.59%) ⬆️
...tor/internal/streaming/SnowflakeSinkServiceV2.java 80.86% <100.00%> (ø)
... and 1 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sfc-gh-rcheng sfc-gh-rcheng changed the title [draft] Rcheng jmx Enable JMX metrics for Snowpipe Streaming Aug 23, 2023
@sfc-gh-rcheng sfc-gh-rcheng marked this pull request as ready for review August 23, 2023 20:12
@sfc-gh-rcheng sfc-gh-rcheng changed the title Enable JMX metrics for Snowpipe Streaming [SNOW-870373] Enable JMX metrics for Snowpipe Streaming Aug 23, 2023
@sfc-gh-xhuang
Copy link
Collaborator

Putting reminder here to update our docs when this is available in the next release:
https://docs.snowflake.com/en/user-guide/kafka-connector-monitor

Copy link
Contributor

@sfc-gh-tzhang sfc-gh-tzhang left a comment

Choose a reason for hiding this comment

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

lgtm!

Comment on lines +73 to +80
/**
* See {@link com.snowflake.kafka.connector.internal.streaming.TopicPartitionChannel} for offset
* description
*/
public static final String OFFSET_PERSISTED_IN_SNOWFLAKE = "persisted-in-snowflake-offset";

public static final String LATEST_CONSUMER_OFFSET = "latest-consumer-offset";

Copy link
Contributor

Choose a reason for hiding this comment

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

They're really useful for our internal debugging, do you think they can be send to Snowhouse as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

100% we can send it through the telemetry service, we currently don't have any telemetry coming out streaming specifically (similar to pipe_usage). planning to add it in a follow up pr if i have time. jira

* @param metricsJmxReporter wrapper class for registering all metrics related to above connector
* and channel
*/
public void registerChannelJMXMetrics(
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, i'm planning to add that in a follow up pr. wanted to keep this one small to just enable JMX, and then add latencies and buffer thresholds later

@sfc-gh-rcheng sfc-gh-rcheng merged commit ef0c907 into master Aug 25, 2023
30 checks passed
@sfc-gh-rcheng sfc-gh-rcheng deleted the rcheng-jmx branch August 25, 2023 00:22
khsoneji pushed a commit to confluentinc/snowflake-kafka-connector that referenced this pull request Oct 12, 2023
khsoneji pushed a commit to confluentinc/snowflake-kafka-connector that referenced this pull request Oct 12, 2023
khsoneji pushed a commit to confluentinc/snowflake-kafka-connector that referenced this pull request Oct 12, 2023
khsoneji pushed a commit to confluentinc/snowflake-kafka-connector that referenced this pull request Oct 12, 2023
EduardHantig pushed a commit to streamkap-com/snowflake-kafka-connector that referenced this pull request Feb 1, 2024
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.

3 participants