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.

Sign up for GitHub

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

Jump to bottom

Flatten Swift telemetry dictionary #27

Merged
merged 2 commits into from
Apr 24, 2024
Merged

Flatten Swift telemetry dictionary #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c0c27ca
flatten inline
Apr 20, 2024
9d9c886
flatten swift telemetry dictionary
Apr 21, 2024
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
2 changes: 0 additions & 2 deletions compiler/cpp/src/thrift/generate/t_java_generator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5292,7 +5292,6 @@ void t_java_generator::generate_java_struct_tuple_writer(ofstream& out, t_struct
}

indent(out) << "oprot.writeBitSet(optionals, " << optional_count << ");" << endl;
int j = 0;
thomasameisel marked this conversation as resolved.
Show resolved Hide resolved
for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
if ((*f_iter)->get_req() == t_field::T_OPTIONAL
|| (*f_iter)->get_req() == t_field::T_OPT_IN_REQ_OUT) {
Expand All @@ -5301,7 +5300,6 @@ void t_java_generator::generate_java_struct_tuple_writer(ofstream& out, t_struct
generate_serialize_field(out, (*f_iter), "struct.", false);
indent_down();
indent(out) << "}" << endl;
j++;
}
}
}
Expand Down
121 changes: 60 additions & 61 deletions compiler/cpp/src/thrift/generate/t_swift_generator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,9 @@ class t_swift_generator : public t_oop_generator {
bool contains_event_name(t_struct* tstruct);
void generate_swift_struct_telemetry_object_extension(ofstream& out, t_struct* tstruct);
void generate_swift_struct_telemetry_event_extension(ofstream& out, t_struct* tstruct);
void telemetry_dictionary_value(ofstream& out, t_type* type, string property_name, string pii_kind);
void telemetry_struct_value(ofstream& out, t_struct* tstruct);
void telemetry_dictionary_value(ofstream& out, string name, t_type* type, string property_name, string pii_kind);
void telemetry_base_type_value(ofstream& out, t_base_type::t_base tbase, string property_name, string pii_kind);
void generate_swift_struct_thrift_extension(ofstream& out,
t_struct* tstruct,
bool is_result,
Expand Down Expand Up @@ -341,7 +343,6 @@ public enum TelemetryValue: Equatable {
case bool(Bool, piiKind: OTPiiKind?)
case int(Int, piiKind: OTPiiKind?)
case double(Double, piiKind: OTPiiKind?)
case dictionary(TelemetryDictionary)
}

)objc";
Expand Down Expand Up @@ -605,12 +606,10 @@ void t_swift_generator::generate_swift_struct(ofstream& out,
vector<t_field*>::const_iterator m_iter;

for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
out << endl;
out << declare_property(*m_iter, is_private) << endl;
out << endl;
}

out << endl;

// init

if (!exclude_empty_init_ || !struct_has_required_fields(tstruct)) {
Expand Down Expand Up @@ -695,8 +694,6 @@ void t_swift_generator::generate_swift_struct_init(ofstream& out,
}

block_close(out);

out << endl;
}

/**
Expand Down Expand Up @@ -864,19 +861,26 @@ void t_swift_generator::generate_swift_struct_telemetry_object_extension(ofstrea
indent(out) << "extension " << tstruct->get_name() << " : TelemetryObject";
block_open(out);

out << endl;

out << indent() << "public func telemetryDictionary() -> TelemetryDictionary";
block_open(out);

out << endl;

if (contains_event_name(tstruct)) {
out << indent() << "var telemetryData = baseProperties()" << endl;
} else {
out << indent() << "var telemetryData = TelemetryDictionary()" << endl;
}

telemetry_struct_value(out, tstruct);

out << indent() << "return telemetryData" << endl;

block_close(out);
block_close(out);

out << endl;
}

void t_swift_generator::telemetry_struct_value(ofstream& out, t_struct* tstruct) {
for (const auto& member : tstruct->get_members()) {
bool optional = field_is_optional(member);

Expand All @@ -890,73 +894,37 @@ void t_swift_generator::generate_swift_struct_telemetry_object_extension(ofstrea
block_open(out);
}

out << indent() << "telemetryData[\"" << member->get_name() << "\"] = ";

string pii_kind = "nil";
std::map<string, string>::iterator it = member->annotations_.find("PIIKind");
if (it != member->annotations_.end()) {
pii_kind = it->second;
}
telemetry_dictionary_value(out, member->get_type(), struct_property_name(member), pii_kind);

out << endl;
telemetry_dictionary_value(out, member->get_name(), member->get_type(), struct_property_name(member), pii_kind);

if (optional) {
block_close(out);
}
}

out << indent() << "return telemetryData" << endl;

block_close(out);
block_close(out);

out << endl;
}

void t_swift_generator::telemetry_dictionary_value(ofstream& out, t_type* type, string property_name, string pii_kind) {
void t_swift_generator::telemetry_dictionary_value(ofstream& out, string name, t_type* type, string property_name, string pii_kind) {
type = get_true_type(type);

if (type->is_base_type()) {
t_base_type::t_base tbase = ((t_base_type*)type)->get_base();
switch (tbase) {
case t_base_type::TYPE_STRING:
out << ".string(" << property_name << ", piiKind: " << pii_kind << ")";
break;
out << indent() << "telemetryData[\"" << name << "\"] = ";

case t_base_type::TYPE_BOOL:
out << ".bool(" << property_name << ", piiKind: " << pii_kind << ")";
break;

case t_base_type::TYPE_I8:
case t_base_type::TYPE_I16:
case t_base_type::TYPE_I32:
case t_base_type::TYPE_I64:
out << ".int(" << property_name << ", piiKind: " << pii_kind << ")";
break;

case t_base_type::TYPE_DOUBLE:
out << ".double(" << property_name << ", piiKind: " << pii_kind << ")";
break;

default:
throw "compiler error: invalid base type " + type->get_name();
break;
}
t_base_type::t_base tbase = ((t_base_type*)type)->get_base();
telemetry_base_type_value(out, tbase, property_name, pii_kind);
out << endl;
} else if (type->is_map()) {
t_map *tmap = (t_map*)type;

out << ".dictionary({";

indent_up();
out << endl;

out << indent() << "var dictionary = TelemetryDictionary()" << endl;
out << indent() << "for (key, value) in " << property_name;

block_open(out);

out << indent() << "dictionary[";
out << indent() << "telemetryData[";

t_type* key_type = get_true_type(tmap->get_key_type());
if (key_type->is_string()) {
Expand All @@ -970,26 +938,57 @@ void t_swift_generator::telemetry_dictionary_value(ofstream& out, t_type* type,

out << "] = ";

telemetry_dictionary_value(out, tmap->get_val_type(), "value", pii_kind);
t_type* value_type = get_true_type(tmap->get_val_type());
if (!value_type->is_base_type()) {
throw "compiler error: unsupported value type for map " + value_type->get_name();
}

t_base_type::t_base tbase = ((t_base_type*)value_type)->get_base();
telemetry_base_type_value(out, tbase, "value", pii_kind);

out << endl;

block_close(out);

out << indent() << "return dictionary" << endl;

block_close(out, false);
out << "())";
} else if (type->is_enum()) {
out << indent() << "telemetryData[\"" << name << "\"] = ";
out << ".string(" << property_name << ".telemetryName(), piiKind: " << pii_kind << ")";
out << endl;
} else if (type->is_struct()) {
out << ".dictionary(" << property_name << ".telemetryDictionary())";
out << indent() << "telemetryData.merge(" << property_name << ".telemetryDictionary()) { _, child in child }";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think we should be warning consumers in the case of a conflict?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have a lint rule here to check for members in sub-structs and maps with enum keys. We're not able to check at compile-time for maps with string keys. Do you have ideas for how we would warn consumers? FWIW we didn't have any logs for this even in Outlook

out << endl;
}
else {
throw "compiler error: invalid type (" + type_name(type) + ") for property \"" + property_name + "\"";
}
}

void t_swift_generator::telemetry_base_type_value(ofstream& out, t_base_type::t_base tbase, string property_name, string pii_kind) {
switch (tbase) {
case t_base_type::TYPE_STRING:
out << ".string(" << property_name << ", piiKind: " << pii_kind << ")";
break;

case t_base_type::TYPE_BOOL:
out << ".bool(" << property_name << ", piiKind: " << pii_kind << ")";
break;

case t_base_type::TYPE_I8:
case t_base_type::TYPE_I16:
case t_base_type::TYPE_I32:
case t_base_type::TYPE_I64:
out << ".int(" << property_name << ", piiKind: " << pii_kind << ")";
break;

case t_base_type::TYPE_DOUBLE:
out << ".double(" << property_name << ", piiKind: " << pii_kind << ")";
break;

default:
throw "compiler error: invalid base type";
break;
}
}

/**
* Generate the TStruct protocol implementation.
*
Expand Down
Loading