Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

fix unknown fields error count metric, change exception message #80

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions src/main/java/com/gojek/beast/converter/RowMapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
import com.gojek.beast.converter.fields.ProtoField;
import com.gojek.beast.exception.UnknownProtoFieldFoundException;
import com.gojek.beast.models.ConfigurationException;
import com.gojek.beast.protomapping.UnknownProtoFields;
import com.gojek.beast.stats.Stats;
import com.gojek.beast.util.DynamicMessageUtil;
import com.google.protobuf.Descriptors;
import com.google.protobuf.DynamicMessage;
import lombok.AllArgsConstructor;
Expand All @@ -34,22 +34,21 @@ public Map<String, Object> map(DynamicMessage message) {
if (mapping == null) {
throw new ConfigurationException("BQ_PROTO_COLUMN_MAPPING is not configured");
}

if (DynamicMessageUtil.isUnknownFieldExist(message)) {
statsClient.count("kafka.error.records.count,type=unknownfields," + statsClient.getBqTags(), 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Log the message with a warn when when unknowfields exists

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will add log

if (failOnUnknownFields) {
throw new UnknownProtoFieldFoundException(message.toString());
}
}

return getMappings(message, mapping);
}

private Map<String, Object> getMappings(DynamicMessage message, ColumnMapping columnMapping) {
if (message == null || columnMapping == null || columnMapping.isEmpty()) {
return new HashMap<>();
}
if (message.getUnknownFields().asMap().size() > 0) {
statsClient.count("kafka.error.records.count,type=unknownfields," + statsClient.getBqTags(), 1);
String serializedUnknownFields = message.getUnknownFields().asMap().keySet().toString();
String serializedMessage = UnknownProtoFields.toString(message.toByteArray());
log.warn(String.format("[%s] unknown fields found in proto [%s], either update mapped protobuf or disable FAIL_ON_UNKNOWN_FIELDS",
serializedUnknownFields, serializedMessage));
if (failOnUnknownFields)
throw new UnknownProtoFieldFoundException(serializedUnknownFields, serializedMessage);
}
Descriptors.Descriptor descriptorForType = message.getDescriptorForType();

Map<String, Object> row = new HashMap<>(columnMapping.size());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
package com.gojek.beast.exception;

public class UnknownProtoFieldFoundException extends RuntimeException {
public UnknownProtoFieldFoundException(String serializedUnknownFields, String serializedMessage) {
super(String.format("[%s] unknown fields found in proto [%s], either update mapped protobuf or disable FAIL_ON_UNKNOWN_FIELDS",
serializedUnknownFields, serializedMessage));
public UnknownProtoFieldFoundException(String serialisedProtoMessage) {
super(String.format("some unknown fields found, please check the published message, update mapped protobuf or disable FAIL_ON_UNKNOWN_FIELDS, full proto message : [%s]", serialisedProtoMessage));
}
}
33 changes: 33 additions & 0 deletions src/main/java/com/gojek/beast/util/DynamicMessageUtil.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.gojek.beast.util;

import com.google.protobuf.DynamicMessage;

import java.util.LinkedList;
import java.util.List;
import java.util.stream.Collectors;

public class DynamicMessageUtil {
public static boolean isUnknownFieldExist(DynamicMessage dynamicMessage) {
if (dynamicMessage == null) {
return false;
}
List<DynamicMessage> dynamicMessageFields = new LinkedList<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

the root message should be there in the dynamicMessageFields

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it automatically add every DynamicMessage to accumulator when traversing the each of the DynamicMessage trees, including the very first node (root)

   private static void collectNestedFields(DynamicMessage root, List<DynamicMessage> accumulator) {
        List<DynamicMessage> nestedChild = root.getAllFields().values().stream()
                .filter(v -> v instanceof DynamicMessage)
                .map(o -> (DynamicMessage) o)
                .collect(Collectors.toList());

        nestedChild.forEach(m -> collectNestedFields(m, accumulator));
        accumulator.add(root);
    }

collectNestedFields(dynamicMessage, dynamicMessageFields);
List<DynamicMessage> messageWithUnknownFields = getMessageWithUnknownFields(dynamicMessageFields);
return messageWithUnknownFields.size() > 0;
}

private static void collectNestedFields(DynamicMessage root, List<DynamicMessage> accumulator) {
List<DynamicMessage> nestedChild = root.getAllFields().values().stream()
.filter(v -> v instanceof DynamicMessage)
.map(o -> (DynamicMessage) o)
.collect(Collectors.toList());

nestedChild.forEach(m -> collectNestedFields(m, accumulator));
accumulator.add(root);
}

private static List<DynamicMessage> getMessageWithUnknownFields(List<DynamicMessage> messages) {
return messages.stream().filter(message -> message.getUnknownFields().asMap().size() > 0).collect(Collectors.toList());
}
}