[Streaming Telemetry 2][SNOW-899866] Enables reportKafkaPartitionStart/Usage telemetry for Streaming #694

sfc-gh-rcheng · 2023-08-25T23:16:45Z

SnowflakeTelemetryChannelStatus becomes a telemetry object, similar to SnowflakeTelemetryPipeUsage
Creates SnowflakeTelemetryChannelCreation, similar to SnowflakeTelemetryPipeCreation
Unifies reportKafkaPartitionStart/Usage code paths and enables telemetry for Streaming
Adds additional code coverage to the TelemetryService
https://snowflakecomputing.atlassian.net/browse/SNOW-899866

src/main/java/com/snowflake/kafka/connector/internal/streaming/TopicPartitionChannel.java

sfc-gh-rcheng · 2023-09-07T19:57:51Z

.../com/snowflake/kafka/connector/internal/streaming/telemetry/SnowflakeTelemetryServiceV2.java

@@ -38,10 +40,9 @@ public SnowflakeTelemetryServiceV2(Connection conn) {
    this.telemetry = TelemetryClient.createTelemetry(conn);
  }

-  @Override


moved reportKafkaPartitionUsage to the parent SnowflakeTelemetryService

sfc-gh-rcheng · 2023-09-07T19:58:49Z

src/main/java/com/snowflake/kafka/connector/internal/telemetry/SnowflakeTelemetryBasicInfo.java


  /**
   * Base Constructor. Accepts a tableName and StageName.
   *
   * @param tableName Checks for Nullability
   */
-  public SnowflakeTelemetryBasicInfo(final String tableName) {
+  public SnowflakeTelemetryBasicInfo(
+      final String tableName, SnowflakeTelemetryService.TelemetryType telemetryType) {


Pass telemetry type here to simplify reportKafkaPartitionUsage/Creation code paths across snowpipe and streaming

sfc-gh-tzhang

Left some comments, PTAL. This is super helpful for us to debug issues without asking for client logs, thanks!

sfc-gh-tzhang · 2023-09-09T00:48:22Z

src/main/java/com/snowflake/kafka/connector/internal/streaming/TopicPartitionChannel.java

@@ -289,6 +290,8 @@ public TopicPartitionChannel(
            this.offsetPersistedInSnowflake,
            this.processedOffset,
            this.latestConsumerOffset);
+    this.telemetryServiceV2.reportKafkaPartitionStart(
+        new SnowflakeTelemetryChannelCreation(this.tableName, this.channelName));


Is this the only place that we're creating channel? If no, we need to either cover all cases or update the metric name to something like partition start

Yes this is the only place we create a channel. There are two constructors for TopicPartitionChannel. The test constructor calls into this constructor, so all TopicPartitionChannel object creations will go through this codepath

I thought we also reopen/create channel when the channel gets invalidated? Not sure if this is the same code path though

The TopicPartitionChannel object does not get recreated when the channel is invalidated, so the current reportKafkaPartitionStart tracks when a new TopicPartitionChannel is created.

Alternatively we could emit a reportKafkaPartitionStart with each channel open, however I don't think this is a good idea because we can track this server side and we already have multiple channel open/close issues that may overload the telemetry service with events.

There are two code paths for opening channels: TopicPartitionChannel constructor and when we recover offset from snowflake (channel invalidation).

.../snowflake/kafka/connector/internal/streaming/telemetry/SnowflakeTelemetryChannelStatus.java

sfc-gh-tzhang · 2023-09-09T00:59:25Z

src/main/java/com/snowflake/kafka/connector/internal/streaming/TopicPartitionChannel.java

@@ -1044,6 +1047,9 @@ private SnowflakeStreamingIngestChannel openChannelForTable() {
  public void closeChannel() {
    try {
      this.channel.close().get();
+
+      // telemetry and metrics
+      this.telemetryServiceV2.reportKafkaPartitionUsage(this.snowflakeTelemetryChannelStatus, true);


Should we send this somewhere periodically instead of just during close?

That would be ideal, but currently we don't have anything running periodically in the background. InsertRows is an option, but I'm worried about overloading the telemetry service since we have no control over how often the customer flushes their buffer

Is it something we plan to add in the future? I think we send the usage for Snowpipe periodically. Otherwise I feel like this is not very useful since there're no usage reported if there is no open/close on the channel

We do not have any plans to add that in the future. It could potentially be a part of the flush service.

Currently we not periodically send Snowpipe usage. reportKafkaPartition() is called on on close and when the cleaner runs, which only runs on the first insert to initialize a pipe

...nowflake/kafka/connector/internal/streaming/telemetry/SnowflakeTelemetryChannelCreation.java

sfc-gh-japatel

mostly looks good, left some questions and suggestions. PTAL and let me know what you think. Thanks!

sfc-gh-japatel · 2023-09-13T19:04:59Z

Lets also verify the data we send to snowflake and confirm we dont change anything in snowpipe telemetry. You can pick any of the above gh action runs and verify the data in telemetry view in snowhouse. (I dont think anything has changed but just to be double sure)

sfc-gh-japatel · 2023-09-13T20:53:02Z

src/main/java/com/snowflake/kafka/connector/internal/telemetry/TelemetryConstants.java

+  public class Streaming {
+    public static final String OFFSET_PERSISTED_IN_SNOWFLAKE = "persisted-in-snowflake-offset";


remove inner class. previous recommendation was to have a separate section/heading in this class which distinguishes between snowpipe and streaming. apologies if I was not clear before.

lets clarify what tp here means. it is not intuitive but hopefully they read code and understand what tp means.

discussed offline, added as a comment section

I'll expand TP -> TopicPartition

sfc-gh-rcheng · 2023-09-13T21:06:50Z

Telemetry in snowhouse

sfc-gh-japatel · 2023-09-13T21:22:46Z

src/main/java/com/snowflake/kafka/connector/internal/metrics/MetricsUtil.java

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


sfc-gh-japatel

Follow up:

merge gates are failing, please ensure they are passing before pushing
doc changes - please create a jira for @sfc-gh-lema (for jmx)

LGTM, otherwise! Good stuff! Thank you

sfc-gh-rcheng · 2023-09-13T22:50:45Z

Jira for documentation changes @sfc-gh-lema

…t/Usage telemetry for Streaming (snowflakedb#694)

sfc-gh-rcheng added 9 commits August 25, 2023 15:18

initial imp

0775e19

autoformatting

273c8d7

actually send telem

340d0d0

autoformatting

ad0a854

refactoer test

ec5352e

added creation

59af12c

autoformatting

038aa7f

make telem nonnull required

ad9b223

autoformatting

f576c0f

sfc-gh-rcheng commented Aug 28, 2023

View reviewed changes

src/main/java/com/snowflake/kafka/connector/internal/streaming/TopicPartitionChannel.java Outdated Show resolved Hide resolved

sfc-gh-rcheng commented Aug 28, 2023

View reviewed changes

src/main/java/com/snowflake/kafka/connector/internal/streaming/TopicPartitionChannel.java Outdated Show resolved Hide resolved

sfc-gh-rcheng commented Aug 28, 2023

View reviewed changes

src/main/java/com/snowflake/kafka/connector/internal/streaming/TopicPartitionChannel.java Outdated Show resolved Hide resolved

sfc-gh-rcheng added 3 commits August 30, 2023 15:31

personal nits

92bf65f

refactoring

16833c6

autoformatting

59786f0

sfc-gh-rcheng changed the title ~~[draft] Rcheng sstelem~~ [Streaming Telemetry 2] Enables reportKafkaPartitionStart/Usage telemetry for Streaming Aug 30, 2023

sfc-gh-rcheng mentioned this pull request Aug 30, 2023

[Streaming Telemetry 1] Prepares TopicPartitionChannel for telemetry #696

Merged

sfc-gh-rcheng added 5 commits September 7, 2023 11:19

merge part 1

1de191c

push for comparison

1bc3dfb

autoformatting

9a281a7

fix tests

ba46a21

autoformatting

25b2220

sfc-gh-rcheng commented Sep 7, 2023

View reviewed changes

sfc-gh-rcheng marked this pull request as ready for review September 7, 2023 21:00

sfc-gh-rcheng requested review from sfc-gh-japatel, sfc-gh-tzhang, sfc-gh-tjones and a team as code owners September 7, 2023 21:00

sfc-gh-tzhang reviewed Sep 9, 2023

View reviewed changes

sfc-gh-japatel reviewed Sep 13, 2023

View reviewed changes

...nowflake/kafka/connector/internal/streaming/telemetry/SnowflakeTelemetryChannelCreation.java Outdated Show resolved Hide resolved

sfc-gh-japatel reviewed Sep 13, 2023

View reviewed changes

sfc-gh-rcheng added 7 commits September 13, 2023 13:26

pr reviews

fbf7719

autoformatting

262ecc6

remove channel open count

766b770

start time -> channel creation time

0faac75

autoformatting

9d5943d

clarify to tpchannel

a47cbcf

autoformatting

09855b2

sfc-gh-japatel reviewed Sep 13, 2023

View reviewed changes

sfc-gh-rcheng added 4 commits September 13, 2023 13:58

no streaming subclass

56dffbd

autoformatting

b2c036d

tp -> topicparition and comment

a951535

autoformatting

947b091

personal nits

d8a8226

sfc-gh-rcheng requested a review from sfc-gh-japatel September 13, 2023 21:10

sfc-gh-japatel reviewed Sep 13, 2023

View reviewed changes

sfc-gh-japatel approved these changes Sep 13, 2023

View reviewed changes

fix npe

6b23cdc

sfc-gh-rcheng changed the title ~~[Streaming Telemetry 2] Enables reportKafkaPartitionStart/Usage telemetry for Streaming~~ [Streaming Telemetry 2][SNOW-899866] Enables reportKafkaPartitionStart/Usage telemetry for Streaming Sep 13, 2023

sfc-gh-rcheng merged commit edb3eaf into master Sep 13, 2023
30 checks passed

sfc-gh-rcheng deleted the rcheng-sstelem branch September 13, 2023 23:02

khsoneji pushed a commit to confluentinc/snowflake-kafka-connector that referenced this pull request Oct 12, 2023

[Streaming Telemetry 2][SNOW-899866] Enables reportKafkaPartitionStar…

24dfd70

…t/Usage telemetry for Streaming (snowflakedb#694)

khsoneji pushed a commit to confluentinc/snowflake-kafka-connector that referenced this pull request Oct 12, 2023

[Streaming Telemetry 2][SNOW-899866] Enables reportKafkaPartitionStar…

f8ce240

…t/Usage telemetry for Streaming (snowflakedb#694)

khsoneji pushed a commit to confluentinc/snowflake-kafka-connector that referenced this pull request Oct 12, 2023

[Streaming Telemetry 2][SNOW-899866] Enables reportKafkaPartitionStar…

bbdfcc5

…t/Usage telemetry for Streaming (snowflakedb#694)

khsoneji pushed a commit to confluentinc/snowflake-kafka-connector that referenced this pull request Oct 12, 2023

[Streaming Telemetry 2][SNOW-899866] Enables reportKafkaPartitionStar…

e3dddca

…t/Usage telemetry for Streaming (snowflakedb#694)

EduardHantig pushed a commit to streamkap-com/snowflake-kafka-connector that referenced this pull request Feb 1, 2024

[Streaming Telemetry 2][SNOW-899866] Enables reportKafkaPartitionStar…

47dc097

…t/Usage telemetry for Streaming (snowflakedb#694)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Streaming Telemetry 2][SNOW-899866] Enables reportKafkaPartitionStart/Usage telemetry for Streaming #694

[Streaming Telemetry 2][SNOW-899866] Enables reportKafkaPartitionStart/Usage telemetry for Streaming #694

sfc-gh-rcheng commented Aug 25, 2023 •

edited

Loading

sfc-gh-rcheng Sep 7, 2023

sfc-gh-rcheng Sep 7, 2023

sfc-gh-tzhang left a comment

sfc-gh-tzhang Sep 9, 2023

sfc-gh-rcheng Sep 11, 2023

sfc-gh-tzhang Sep 12, 2023

sfc-gh-rcheng Sep 12, 2023 •

edited

Loading

sfc-gh-tzhang Sep 9, 2023

sfc-gh-rcheng Sep 11, 2023

sfc-gh-tzhang Sep 12, 2023

sfc-gh-rcheng Sep 12, 2023

sfc-gh-japatel left a comment

sfc-gh-japatel commented Sep 13, 2023

sfc-gh-japatel Sep 13, 2023 •

edited

Loading

sfc-gh-rcheng Sep 13, 2023

sfc-gh-rcheng commented Sep 13, 2023

sfc-gh-japatel Sep 13, 2023

sfc-gh-japatel left a comment •

edited

Loading

sfc-gh-rcheng commented Sep 13, 2023

		public class Streaming {
		public static final String OFFSET_PERSISTED_IN_SNOWFLAKE = "persisted-in-snowflake-offset";

[Streaming Telemetry 2][SNOW-899866] Enables reportKafkaPartitionStart/Usage telemetry for Streaming #694

[Streaming Telemetry 2][SNOW-899866] Enables reportKafkaPartitionStart/Usage telemetry for Streaming #694

Conversation

sfc-gh-rcheng commented Aug 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-tzhang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-rcheng Sep 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-japatel left a comment

Choose a reason for hiding this comment

sfc-gh-japatel commented Sep 13, 2023

sfc-gh-japatel Sep 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-rcheng commented Sep 13, 2023

Choose a reason for hiding this comment

sfc-gh-japatel left a comment • edited Loading

Choose a reason for hiding this comment

sfc-gh-rcheng commented Sep 13, 2023

sfc-gh-rcheng commented Aug 25, 2023 •

edited

Loading

sfc-gh-rcheng Sep 12, 2023 •

edited

Loading

sfc-gh-japatel Sep 13, 2023 •

edited

Loading

sfc-gh-japatel left a comment •

edited

Loading