Introduce KafkaRecordEntity to support Kafka headers in InputFormats #10730

xvrl · 2021-01-05T20:27:39Z

Today Kafka message support in streaming indexing tasks is limited to
message values, and does not provide a way to expose Kafka headers,
timestamps, or keys, which may be of interest to more specialized
Druid input formats. For instance, Kafka headers may be used to indicate
payload format/encoding or additional metadata, and timestamps are often
omitted from values in Kafka streams applications, since they are
included in the record.

This change proposes to introduce KafkaRecordEntity as InputEntity,
which would give input formats full access to the underlying Kafka record,
including headers, key, timestamps. It would also open access to low-level
information such as topic, partition, offset if needed.

KafkaEntity is a subclass of ByteEntity for backwards compatibility with
existing input formats, and to avoid introducing unnecessary complexity
for Kinesis indexing tasks.

Today Kafka message support in streaming indexing tasks is limited to message values, and does not provide a way to expose Kafka headers, timestamps, or keys, which may be of interest to more specialized Druid input formats. For instance, Kafka headers may be used to indicate payload format/encoding or additional metadata, and timestamps are often omitted from values in Kakfa streams applications, since they are included in the record. This change proposes to introduce KafkaRecordEntity as InputEntity, which would give input formats full access to the underlying Kafka record, including headers, key, timestamps. It would also open access to low-level information such as topic, partition, offset if needed. KafkaEntity is a subclass of ByteEntity for backwards compatbility with existing input formats, and to avoid introducing unnecessary complexity for Kinesis indexing tasks.

xvrl · 2021-01-06T03:14:49Z

...afka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaRecordSupplier.java

      polledRecords.add(new OrderedPartitionableRecord<>(
          record.topic(),
          record.partition(),
          record.offset(),
-          record.value() == null ? null : ImmutableList.of(record.value())
+          ImmutableList.of(new KafkaRecordEntity(record))


note, today we skip records with null values, so we should decide if/how we want to allow passing those through. I'm thinking we could add a config option if needed that would expose them as empty byte arrays for backwards compatibility in ByteEntity, but allow the full record in KafkaRecordEntity. However, I have not found many use-cases for null values, so we can decide to punt on this and add this option later if needed.

I think it is fine as long as it doesn't cause an unhandled NPE somewhere downstream and just counts as unparseable record for existing pipelines.

I kept reverted to the original behavior for now

himanshug

LGTM overall, few minor comments

himanshug · 2021-01-06T23:57:49Z

...afka-indexing-service/src/main/java/org/apache/druid/data/input/kafka/KafkaRecordEntity.java

+    this.record = record;
+  }
+
+  public ConsumerRecord<byte[], byte[]> getRecord()


not used anywhere , is this only to allow writing extensions with custom InputFormat which can take advantage of the extra metadata available ?

that's correct. We could have a sample InputFormat for documentation purposes, though I'm not sure if there is much value. A more generic input format for Kafka that wraps multiple input-formats would require a lot more thought, and I couldn't think of a one-size-fits-all approach that seemed very useful or didn't have additional complexities to deal with.

himanshug · 2021-01-07T01:49:24Z

...afka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaRecordSupplier.java

      polledRecords.add(new OrderedPartitionableRecord<>(
          record.topic(),
          record.partition(),
          record.offset(),
-          record.value() == null ? null : ImmutableList.of(record.value())
+          ImmutableList.of(new KafkaRecordEntity(record))


I think it is fine as long as it doesn't cause an unhandled NPE somewhere downstream and just counts as unparseable record for existing pipelines.

himanshug · 2021-01-07T01:54:17Z

...rc/main/java/org/apache/druid/indexing/seekablestream/common/OrderedPartitionableRecord.java

@@ -55,7 +54,7 @@ public OrderedPartitionableRecord(
    this.stream = stream;
    this.partitionId = partitionId;
    this.sequenceNumber = sequenceNumber;
-    this.data = data == null ? ImmutableList.of() : data;
+    this.data = data;


why remove the null check and conversion to empty list ?

I reverted this change after seeing that we rely on null in a bunch of places.

xvrl · 2021-01-07T17:50:23Z

...afka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaRecordSupplier.java

      polledRecords.add(new OrderedPartitionableRecord<>(
          record.topic(),
          record.partition(),
          record.offset(),
-          record.value() == null ? null : ImmutableList.of(record.value())
+          record.value() == null ? null : ImmutableList.of(new KafkaRecordEntity(record))


@himanshug I reverted this change to keep the existing behavior

We should put in a comment or javadoc noting that records with null values will be skipped, even if the other stuff (keys, timestamp, headers) are not null.

Here's a suggestion:

Add a note to the javadoc of KafkaRecordEntity mentioning that these entities won't be generated for messages with null values

Add a null-value check to the constructor of KafkaRecordEntity enforcing that comment

done, KafkaRecordEntity will already barf on null values when instantiating ByteEntity

xvrl · 2021-01-07T18:47:46Z

...-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/KinesisRecordSupplier.java

-                records.remainingCapacity(),
-                currRecord.getData().stream().map(StringUtils::fromUtf8).collect(Collectors.toList())
-            );
+            if (log.isTraceEnabled()) {


I added this check, to avoid unnecessarily deserializing all the bytes as strings

Nice 😄

I bet it will speed up Kinesis ingestion a bit.

let's hope so, using ByteEntities directly should also avoid having to copy around byte arrays

xvrl · 2021-01-07T18:48:48Z

...-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/KinesisRecordSupplier.java

-                currRecord.getPartitionId(),
-                currRecord.getSequenceNumber(),
-                records.remainingCapacity(),
-                currRecord.getData().stream().map(StringUtils::fromUtf8).collect(Collectors.toList())


I'm not even sure this is safe to do. Some byte arrays could technically be invalid UTF-8 sequences, which would lead to exceptions when trying to log values.

It's OK, because fromUtf8 doesn't throw exceptions, it just replaces invalid sequences with the replacement character. Although the log messages will look very weird.

gianm

Elegant solution!

gianm · 2021-01-07T16:58:22Z

...afka-indexing-service/src/main/java/org/apache/druid/data/input/kafka/KafkaRecordEntity.java

+import org.apache.druid.data.input.impl.ByteEntity;
+import org.apache.kafka.clients.consumer.ConsumerRecord;
+
+public class KafkaRecordEntity extends ByteEntity


Could you please add javadocs explaining how this class is meant to be used? Something like:

A ByteEntity generated by {@link KafkaRecordSupplier} and fed to any {@link InputFormat} used by a Kafka indexing tasks. It can be used as a regular ByteEntity, in which case the Kafka message value is returned. But the {@link #getRecord} method also allows Kafka-aware InputFormat implementations to read the full Kafka message, including headers, key, and timestamp.

This functionality is not yet exposed through any built-in InputFormats, but is available for use in extensions.

gianm · 2021-01-07T20:14:37Z

...afka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaRecordSupplier.java

      polledRecords.add(new OrderedPartitionableRecord<>(
          record.topic(),
          record.partition(),
          record.offset(),
-          record.value() == null ? null : ImmutableList.of(record.value())
+          record.value() == null ? null : ImmutableList.of(new KafkaRecordEntity(record))


We should put in a comment or javadoc noting that records with null values will be skipped, even if the other stuff (keys, timestamp, headers) are not null.

Here's a suggestion:

Add a note to the javadoc of KafkaRecordEntity mentioning that these entities won't be generated for messages with null values

Add a null-value check to the constructor of KafkaRecordEntity enforcing that comment

gianm · 2021-01-07T20:16:17Z

...-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/KinesisRecordSupplier.java

-                records.remainingCapacity(),
-                currRecord.getData().stream().map(StringUtils::fromUtf8).collect(Collectors.toList())
-            );
+            if (log.isTraceEnabled()) {


Nice 😄

I bet it will speed up Kinesis ingestion a bit.

gianm · 2021-01-07T20:17:21Z

...-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/KinesisRecordSupplier.java

-                currRecord.getPartitionId(),
-                currRecord.getSequenceNumber(),
-                records.remainingCapacity(),
-                currRecord.getData().stream().map(StringUtils::fromUtf8).collect(Collectors.toList())


It's OK, because fromUtf8 doesn't throw exceptions, it just replaces invalid sequences with the replacement character. Although the log messages will look very weird.

gianm · 2021-01-07T20:29:27Z

...kafka-indexing-service/src/test/java/org/apache/druid/indexing/kafka/KafkaIndexTaskTest.java

-        new ProducerRecord<>(topic, 0, null, jb("2011", "d", "y", "10", "20.0", "1.0")),
-        new ProducerRecord<>(topic, 0, null, jb("2011", "e", "y", "10", "20.0", "1.0")),
-        new ProducerRecord<>(topic, 0, null, jb("246140482-04-24T15:36:27.903Z", "x", "z", "10", "20.0", "1.0")),
+        new ProducerRecord<>(topic, 0, null, jbb("2008", "a", "y", "10", "20.0", "1.0")),


Unless I missed it — could you add a test that uses a custom InputFormat and verifies that something other than the value can be read properly?

makes sense, will add that

gianm · 2021-01-07T20:32:24Z

...rc/main/java/org/apache/druid/indexing/seekablestream/common/OrderedPartitionableRecord.java

@@ -74,7 +74,7 @@ public SequenceOffsetType getSequenceNumber()
  }

  @NotNull
-  public List<byte[]> getData()
+  public List<? extends ByteEntity> getData()


This change means we're returning a thing that wraps a ByteBuffer instead of a byte[], which opens up potential issues due to callers handling the position and limit wrong. What are the expectations? Is it ok for callers to change the position of the underlying buffer or should they refrain? If they do change it, does that cause problems?

(For example: do we have situations where something calls getData() to parse a value, and that updates the position, and then later on getData() is called again for some other purpose like logging? If so — that'd break as a result of this change.)

Whatever the case, the javadoc for this method should describe the conclusion.

I did encounter some of this in test cases, where we would call getData on a record multiple times, causing issues. In general however, an InputEntity doesn't support being read multiple times, e.g. for ByteEntity, once it's been read via InputEntity.open(), the underlying buffer would have been exhausted and return an empty stream on subsequent calls to open().

Except for one logging case, all the calls to OrderedPartitionableRecord.getData() I could find would only be executed once per record, for the purpose of exposing InputEntities to parsers, which should ensure the buffers only get read once or treated appropriately as needed. I updated the logging case to make sure it wouldn't modify the buffer position.

gianm

LGTM

xvrl · 2021-01-08T21:01:04Z

looks like tests are failing due to insufficient diff coverage for some lines. Many of them appear to be where we introduced additional type parameters. @gianm what's the guidance here?

jihoonson

LGTM. For test coverage failure, it doesn't seem worth fixing it because they are about the new type parameter and avoiding trace-level logging. For LGTM failure, I think it will pass once you merge master into this branch (fixed in #10738).

gianm · 2021-01-08T21:12:02Z

looks like tests are failing due to insufficient diff coverage for some lines. Many of them appear to be where we introduced additional type parameters. @gianm what's the guidance here?

What I've done is either,

Add tests if it's something that really should be tested
Add a suppression if it's something that really shouldn't be tested
Ignore the coverage check if it's something that could be tested, but it isn't that important, and we want to skip it for now. (It's a diff-oriented check, so ignoring it won't affect future patches.)

xvrl · 2021-01-09T00:03:56Z

ok I will merge it as is then. Adding specific tests for the trace logging seems a little overkill. We can add it later if we feel strongly about it.

…pache#10730) Today Kafka message support in streaming indexing tasks is limited to message values, and does not provide a way to expose Kafka headers, timestamps, or keys, which may be of interest to more specialized Druid input formats. For instance, Kafka headers may be used to indicate payload format/encoding or additional metadata, and timestamps are often omitted from values in Kafka streams applications, since they are included in the record. This change proposes to introduce KafkaRecordEntity as InputEntity, which would give input formats full access to the underlying Kafka record, including headers, key, timestamps. It would also open access to low-level information such as topic, partition, offset if needed. KafkaEntity is a subclass of ByteEntity for backwards compatibility with existing input formats, and to avoid introducing unnecessary complexity for Kinesis indexing tasks.

### Description Today we ingest a number of high cardinality metrics into Druid across dimensions. These metrics are rolled up on a per minute basis, and are very useful when looking at metrics on a partition or client basis. Events is another class of data that provides useful information about a particular incident/scenario inside a Kafka cluster. Events themselves are carried inside kafka payload, but nonetheless there are some very useful metadata that is carried in kafka headers that can serve as useful dimension for aggregation and in turn bringing better insights. PR(#10730) introduced support of Kafka headers in InputFormats. We still need an input format to parse out the headers and translate those into relevant columns in Druid. Until that’s implemented, none of the information available in the Kafka message headers would be exposed. So first there is a need to write an input format that can parse headers in any given format(provided we support the format) like we parse payloads today. Apart from headers there is also some useful information present in the key portion of the kafka record. We also need a way to expose the data present in the key as druid columns. We need a generic way to express at configuration time what attributes from headers, key and payload need to be ingested into druid. We need to keep the design generic enough so that users can specify different parsers for headers, key and payload. This PR is designed to solve the above by providing wrapper around any existing input formats and merging the data into a single unified Druid row. Lets look at a sample input format from the above discussion "inputFormat": { "type": "kafka", // New input format type "headerLabelPrefix": "kafka.header.", // Label prefix for header columns, this will avoid collusions while merging columns "recordTimestampLabelPrefix": "kafka.", // Kafka record's timestamp is made available in case payload does not carry timestamp "headerFormat": // Header parser specifying that values are of type string { "type": "string" }, "valueFormat": // Value parser from json parsing { "type": "json", "flattenSpec": { "useFieldDiscovery": true, "fields": [...] } }, "keyFormat": // Key parser also from json parsing { "type": "json" } } Since we have independent sections for header, key and payload, it will enable parsing each section with its own parser, eg., headers coming in as string and payload as json. KafkaInputFormat will be the uber class extending inputFormat interface and will be responsible for creating individual parsers for header, key and payload, blend the data resolving conflicts in columns and generating a single unified InputRow for Druid ingestion. "headerFormat" will allow users to plug parser type for the header values and will add default header prefix as "kafka.header."(can be overridden) for attributes to avoid collision while merging attributes with payload. Kafka payload parser will be responsible for parsing the Value portion of the Kafka record. This is where most of the data will come from and we should be able to plugin existing parser. One thing to note here is that if batching is performed, then the code is augmenting header and key values to every record in the batch. Kafka key parser will handle parsing Key portion of the Kafka record and will ingest the Key with dimension name as "kafka.key". ## KafkaInputFormat Class: This is the class that orchestrates sending the consumerRecord to each parser, retrieve rows, merge the columns into one final row for Druid consumption. KafkaInputformat should make sure to release the resources that gets allocated as a part of reader in CloseableIterator<InputRow> during normal and exception cases. During conflicts in dimension/metrics names, the code will prefer dimension names from payload and ignore the dimension either from headers/key. This is done so that existing input formats can be easily migrated to this new format without worrying about losing information.

Today we ingest a number of high cardinality metrics into Druid across dimensions. These metrics are rolled up on a per minute basis, and are very useful when looking at metrics on a partition or client basis. Events is another class of data that provides useful information about a particular incident/scenario inside a Kafka cluster. Events themselves are carried inside kafka payload, but nonetheless there are some very useful metadata that is carried in kafka headers that can serve as useful dimension for aggregation and in turn bringing better insights. PR(apache#10730) introduced support of Kafka headers in InputFormats. We still need an input format to parse out the headers and translate those into relevant columns in Druid. Until that’s implemented, none of the information available in the Kafka message headers would be exposed. So first there is a need to write an input format that can parse headers in any given format(provided we support the format) like we parse payloads today. Apart from headers there is also some useful information present in the key portion of the kafka record. We also need a way to expose the data present in the key as druid columns. We need a generic way to express at configuration time what attributes from headers, key and payload need to be ingested into druid. We need to keep the design generic enough so that users can specify different parsers for headers, key and payload. This PR is designed to solve the above by providing wrapper around any existing input formats and merging the data into a single unified Druid row. Lets look at a sample input format from the above discussion "inputFormat": { "type": "kafka", // New input format type "headerLabelPrefix": "kafka.header.", // Label prefix for header columns, this will avoid collusions while merging columns "recordTimestampLabelPrefix": "kafka.", // Kafka record's timestamp is made available in case payload does not carry timestamp "headerFormat": // Header parser specifying that values are of type string { "type": "string" }, "valueFormat": // Value parser from json parsing { "type": "json", "flattenSpec": { "useFieldDiscovery": true, "fields": [...] } }, "keyFormat": // Key parser also from json parsing { "type": "json" } } Since we have independent sections for header, key and payload, it will enable parsing each section with its own parser, eg., headers coming in as string and payload as json. KafkaInputFormat will be the uber class extending inputFormat interface and will be responsible for creating individual parsers for header, key and payload, blend the data resolving conflicts in columns and generating a single unified InputRow for Druid ingestion. "headerFormat" will allow users to plug parser type for the header values and will add default header prefix as "kafka.header."(can be overridden) for attributes to avoid collision while merging attributes with payload. Kafka payload parser will be responsible for parsing the Value portion of the Kafka record. This is where most of the data will come from and we should be able to plugin existing parser. One thing to note here is that if batching is performed, then the code is augmenting header and key values to every record in the batch. Kafka key parser will handle parsing Key portion of the Kafka record and will ingest the Key with dimension name as "kafka.key". This is the class that orchestrates sending the consumerRecord to each parser, retrieve rows, merge the columns into one final row for Druid consumption. KafkaInputformat should make sure to release the resources that gets allocated as a part of reader in CloseableIterator<InputRow> during normal and exception cases. During conflicts in dimension/metrics names, the code will prefer dimension names from payload and ignore the dimension either from headers/key. This is done so that existing input formats can be easily migrated to this new format without worrying about losing information. (cherry picked from commit ad6609a)

…load parsing (apache#11630) (#52) Today we ingest a number of high cardinality metrics into Druid across dimensions. These metrics are rolled up on a per minute basis, and are very useful when looking at metrics on a partition or client basis. Events is another class of data that provides useful information about a particular incident/scenario inside a Kafka cluster. Events themselves are carried inside kafka payload, but nonetheless there are some very useful metadata that is carried in kafka headers that can serve as useful dimension for aggregation and in turn bringing better insights. PR(apache#10730) introduced support of Kafka headers in InputFormats. We still need an input format to parse out the headers and translate those into relevant columns in Druid. Until that’s implemented, none of the information available in the Kafka message headers would be exposed. So first there is a need to write an input format that can parse headers in any given format(provided we support the format) like we parse payloads today. Apart from headers there is also some useful information present in the key portion of the kafka record. We also need a way to expose the data present in the key as druid columns. We need a generic way to express at configuration time what attributes from headers, key and payload need to be ingested into druid. We need to keep the design generic enough so that users can specify different parsers for headers, key and payload. This PR is designed to solve the above by providing wrapper around any existing input formats and merging the data into a single unified Druid row. Lets look at a sample input format from the above discussion "inputFormat": { "type": "kafka", // New input format type "headerLabelPrefix": "kafka.header.", // Label prefix for header columns, this will avoid collusions while merging columns "recordTimestampLabelPrefix": "kafka.", // Kafka record's timestamp is made available in case payload does not carry timestamp "headerFormat": // Header parser specifying that values are of type string { "type": "string" }, "valueFormat": // Value parser from json parsing { "type": "json", "flattenSpec": { "useFieldDiscovery": true, "fields": [...] } }, "keyFormat": // Key parser also from json parsing { "type": "json" } } Since we have independent sections for header, key and payload, it will enable parsing each section with its own parser, eg., headers coming in as string and payload as json. KafkaInputFormat will be the uber class extending inputFormat interface and will be responsible for creating individual parsers for header, key and payload, blend the data resolving conflicts in columns and generating a single unified InputRow for Druid ingestion. "headerFormat" will allow users to plug parser type for the header values and will add default header prefix as "kafka.header."(can be overridden) for attributes to avoid collision while merging attributes with payload. Kafka payload parser will be responsible for parsing the Value portion of the Kafka record. This is where most of the data will come from and we should be able to plugin existing parser. One thing to note here is that if batching is performed, then the code is augmenting header and key values to every record in the batch. Kafka key parser will handle parsing Key portion of the Kafka record and will ingest the Key with dimension name as "kafka.key". This is the class that orchestrates sending the consumerRecord to each parser, retrieve rows, merge the columns into one final row for Druid consumption. KafkaInputformat should make sure to release the resources that gets allocated as a part of reader in CloseableIterator<InputRow> during normal and exception cases. During conflicts in dimension/metrics names, the code will prefer dimension names from payload and ignore the dimension either from headers/key. This is done so that existing input formats can be easily migrated to this new format without worrying about losing information. (cherry picked from commit ad6609a)

xvrl added Proposal Area - Streaming Ingestion labels Jan 5, 2021

xvrl requested review from himanshug and gianm January 5, 2021 20:27

xvrl commented Jan 6, 2021

View reviewed changes

fix type bounds in tests

b687f16

xvrl force-pushed the kafka-record-entity branch from 265cbfe to b687f16 Compare January 6, 2021 04:33

xvrl added 5 commits January 6, 2021 10:00

fix checkstyle

809de31

restore null message handling

0b0911e

remove unused method

a42d71d

fix kinesis tests reading records multiple times

36e51e0

ensure logging does not modify buffer position

876d455

himanshug reviewed Jan 7, 2021

View reviewed changes

xvrl added 2 commits January 7, 2021 09:15

make checkstyle happy

99c37e0

add javadocs

abd1425

xvrl commented Jan 7, 2021

View reviewed changes

gianm reviewed Jan 7, 2021

View reviewed changes

xvrl added 3 commits January 7, 2021 18:22

clarify the behavior of some key methods

f35f7d4

add kafka input format test

025822b

more checkstyle happiness

680709b

gianm approved these changes Jan 8, 2021

View reviewed changes

xvrl added 2 commits January 8, 2021 00:22

fix OrderedPartitionableRecord equality

9ef2256

fix typo

c70418d

jihoonson approved these changes Jan 8, 2021

View reviewed changes

xvrl removed the Proposal label Jan 8, 2021

xvrl merged commit 118b501 into apache:master Jan 9, 2021

xvrl deleted the kafka-record-entity branch January 9, 2021 00:04

jihoonson added this to the 0.21.0 milestone Jul 15, 2021

lokesh-lingarajan mentioned this pull request Aug 24, 2021

Kafka Input Format for headers, key and payload parsing. #11630

Merged

7 tasks

lokesh-lingarajan mentioned this pull request Oct 12, 2021

cherry pick #11630: Kafka Input Format for headers, key and payload parsing confluentinc/druid#52

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce KafkaRecordEntity to support Kafka headers in InputFormats #10730

Introduce KafkaRecordEntity to support Kafka headers in InputFormats #10730

xvrl commented Jan 5, 2021

xvrl Jan 6, 2021

himanshug Jan 7, 2021

xvrl Jan 8, 2021

himanshug left a comment

himanshug Jan 6, 2021

xvrl Jan 7, 2021 •

edited

himanshug Jan 7, 2021

himanshug Jan 7, 2021

xvrl Jan 7, 2021

xvrl Jan 7, 2021

gianm Jan 7, 2021

xvrl Jan 8, 2021

xvrl Jan 7, 2021

gianm Jan 7, 2021

xvrl Jan 8, 2021

xvrl Jan 7, 2021

gianm Jan 7, 2021

xvrl Jan 8, 2021

gianm left a comment

gianm Jan 7, 2021

gianm Jan 7, 2021

gianm Jan 7, 2021

gianm Jan 7, 2021

gianm Jan 7, 2021

xvrl Jan 7, 2021

xvrl Jan 8, 2021

gianm Jan 7, 2021 •

edited

xvrl Jan 7, 2021 •

edited

gianm left a comment

xvrl commented Jan 8, 2021

jihoonson left a comment

gianm commented Jan 8, 2021

xvrl commented Jan 9, 2021

Introduce KafkaRecordEntity to support Kafka headers in InputFormats #10730

Introduce KafkaRecordEntity to support Kafka headers in InputFormats #10730

Conversation

xvrl commented Jan 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

himanshug left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xvrl Jan 7, 2021 • edited

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gianm 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

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

gianm Jan 7, 2021 • edited

Choose a reason for hiding this comment

xvrl Jan 7, 2021 • edited

Choose a reason for hiding this comment

gianm left a comment

Choose a reason for hiding this comment

xvrl commented Jan 8, 2021

jihoonson left a comment

Choose a reason for hiding this comment

gianm commented Jan 8, 2021

xvrl commented Jan 9, 2021

xvrl Jan 7, 2021 •

edited

gianm Jan 7, 2021 •

edited

xvrl Jan 7, 2021 •

edited