Support ext.metadata attributes
This feature is related to OTEL asks #376 #378.
Common Schema issues: https://github.com/microsoft/common-schema/issues/43 https://github.com/microsoft/common-schema/pull/134
Currently Bond-protocol SDK has no way of expressing ext.metadata attributes. There is already API surface to populate ext.metadata.privTags attribute - described here: https://github.com/microsoft/cpp_client_telemetry/blob/master/docs/Privacy-Settings.md But this API is unique to SDK running on Windows 10 in UTC mode only.
Feature Proposal below is incrementally building on top of existing concepts with no API change, proposing to extend the existing UTC-only concept to Bond path.
Part 1. Special aliases for metadata attributes
EventProperty attributes with matching names get mapped to ext.metadata* :
- "ext.metadata.myEventTag"
- "ext.metadata.myCloud.FooBarTag"
- COMMONFIELDS_EVENT_PRIVTAGS
- COMMONFIELDS_METADATA_VIEWINGEXTRA1
- complete list of reserved fields here: https://github.com/microsoft/cpp_client_telemetry/blob/7fe117c927c1e891e44cba7b02a2cf9a039a6f38/lib/include/public/CommonFields.h#L64
Remapping rules - what gets remapped from EventProperties into ext.metadata :
- any property name 'alias' that is known to be metadata at compile time from the list above.
- any property name that starts with ext.metadata. or Metadata.
How we do this today in UTC module COMMONFIELDS_METADATA:
#define COMMONFIELDS_METADATA_VIEWINGPRODUCERID "MetaData.ViewingProducerId"
#define COMMONFIELDS_METADATA_VIEWINGCATEGORY "MetaData.ViewingCategory"
#define COMMONFIELDS_METADATA_VIEWINGPAYLOADDECODERPATH "MetaData.ViewingPayloadDecoderPath"
#define COMMONFIELDS_METADATA_VIEWINGPAYLOADENCODEDFIELDNAME "MetaData.ViewingPayloadEncodedFieldName"
#define COMMONFIELDS_METADATA_VIEWINGEXTRA1 "MetaData.ViewingExtra1"
#define COMMONFIELDS_METADATA_VIEWINGEXTRA2 "MetaData.ViewingExtra2"
#define COMMONFIELDS_METADATA_VIEWINGEXTRA3 "MetaData.ViewingExtra3"
we express Win 10 UTC PDT tags and DDT viewer category tags as field on EventProperties. Subsequently the UTC module is transforming/remapping/unflattening from flat properties list into 'magic' property known to UTC and DDV. The list can be extended to support more metadata attributes in newer SDKs.
Part 2. DataCategory = DataCategory_Metadata setter
Slightly better-structured (but less idiomatic, less 'fluent'!) non-mutually exclusive alternative is to allow populating any metadata property using SetProperty overload with 4th parameter set here:
void SetProperty(
const std::string& name,
const std::string& value,
PiiKind piiKind = PiiKind_None,
DataCategory category = DataCategory_PartC // <--- this parameter
);
DataCategory enum currently has these values:
- DataCategory_PartB - populate Part B data property.
- DataCategory_PartC - populate Part C data property.
Can be supplemented with the following new category:
- DataCategory_Metadata - populate metadata property.
Usage example:
event.SetProperty("MyCustomMetadata", "value", PiiKind_None, DataCategory_Metadata);
event.SetProperty("privTags", 1, PiiKind_None, DataCategory_Metadata);
event.SetProperty("level", 2, PiiKind_None, DataCategory_Metadata);
Note there is no need to prefix the property with ext.metadata in this notation: 4th parameter allows to explicitly scope the attribute to metadata property bag. That would allow to populate "metadata" extension without any drastic API surface changes, i.e. simply by expanding the existing DataCategory enum to also cover metadata attributes.
This approach is slightly "more structured". While the Option 1 - relying on existing 'magic alias' is:
-
more natural and user-friendly, as it explicitly allows referring to metadata field by its natural name, e.g. ext.metadata.MyField
-
yields better performance: initializer list populates collection once without needing an extra function call, no extra memcpy
Both of these are reasonable approaches that I agree with to a certain degree.
One thing to consider is what each metadata property is being used and where it applies. For example, for any of the Privacy Metadata, as far as I am aware, is applicable at event level, not field level. Then, some of the COMMONFIELDS_METADATA_VIEWING* fields are specific to Edge as per their design (COMMONFIELDS_METADATA_VIEWINGPAYLOADDECODERPATH, etc.).
I'd propose a mix of Part 1 and 2, as having setters for various metadata fields. For example, for Privacy Tag and Privacy Level, we can have the following setters on EventProperties.
void SetPrivacyTag(uint64_t privTag)
{
//Validate privTag;
event.SetProperty("ext.metadata.PrivTags", privTag);
}
void SetPrivacyLevel(uint32_t privLevel)
{
//Validate privLevel;
event.SetProperty("ext.metadata.PrivLevel", privLevel);
}
This should ensure that we are keeping the flexibility with the properties, while also ensuring structure for the fields as we need. Additionally, by not storing the field location (ext.metadata.Priv*) in a CommonFields macro, we can work towards ensuring that the fields are set using their dedicated setters that can perform metadata validation and sanitization as needed. For example, say, PrivTags and PrivLevel can only be set using the sample methods shared above.
Another quality of life improvement would be to further extend any critical metadata to be part of LogEvent when event properties are not being sent to keep the instrumentation code simple. A proposed LogEvent overload would be:
virtual void LogEvent(std::string const& name, uint64_t privTag, uint32_t privLevel) = 0;
Of course, we can further improve upon this by having a Metadata struct that can be passed to LogEvent with any metadata that we define:
struct EventMetadata
{
uint64_t PrivTags;
uint32_t PrivLevel;
bool MyCustomMetadata;
};
virtual void LogEvent(std::string const& name, EventMetadata const& customMetadata) = 0;
//Usage Example:
LogEvent("MyEvent", EventMetadata{PDT_1 ,PDL_2, true });
We avoided having a dedicated getter / setter for all of DDV metadata and PDT tags before. To address your concern with some events not having metadata - we can extend the SetContext API to allow expressing metadata tags that by-default apply to all events emitted for specific ILogger. Then if you need to adjust the level of one event, you can individually specify the level inline using SetProperty or static initializer list, same as for any other event property.
Another question I have: since we introduced SetLevel API as one-off for Office, making API surface complex enough, which already populates COMMONFIELDS_EVENT_LEVEL - this is described in the doc.. Do you really need yet another dimension, or can you incrementally build upon that existing "Event Level" - extend the set to cover additional levels you need? SetLevel API has been designed extensible, with an expectation that every customer may have their own sets of levels.
I do not exactly see why we need to solve this by changing API, introducing new type and LogEvent overload, where we solved similar / same conceptual issues without any API changes before.
Removing milestone, as there are no customers blocked by this.
@sid-dahiya - should we be closing it, since there's a new alternate proposal in Common Schema repo?
I'm reassigning this to @sid-dahiya since I no longer actively work on 1DS. Feel free to close or move elsewhere.