Derived EventSource type fails to generate ETW manifest when a member function contains Nullable parameter

[noahfalk]

See dotnet/corefx#32777 (comment) for initial details.

[noahfalk]

This issue is only tangentially related to EventListener actually (I'm going to update the title). It is caused when an EventSource attempts to generate an ETW manifest by reflecting over the arguments to its event logging methods. The reflection code doesn't know how to translate Nullable into a schema. The error occurs here:

System.Private.CoreLib.dll!System.Diagnostics.Tracing.ManifestBuilder.ManifestError(string msg, bool runtimeCritical) Line 5479	C#
System.Private.CoreLib.dll!System.Diagnostics.Tracing.ManifestBuilder.GetTypeNameHelper(System.Type type) Line 188	C#
System.Private.CoreLib.dll!System.Diagnostics.Tracing.ManifestBuilder.GetTypeName(System.Type type) Line 5902	C#
System.Private.CoreLib.dll!System.Diagnostics.Tracing.ManifestBuilder.AddEventParameter(System.Type type, string name) Line 5371	C#
System.Private.CoreLib.dll!System.Diagnostics.Tracing.EventSource.CreateManifestAndDescriptors(System.Type eventSourceType, string eventSourceDllName, System.Diagnostics.Tracing.EventSource source, System.Diagnostics.Tracing.EventManifestOptions flags) Line 3194	C#
System.Private.CoreLib.dll!System.Diagnostics.Tracing.EventSource.EnsureDescriptorsInitialized() Line 2705	C#
System.Private.CoreLib.dll!System.Diagnostics.Tracing.EventSource.DoCommand(System.Diagnostics.Tracing.EventCommandEventArgs commandArgs) Line 2485	C#
System.Private.CoreLib.dll!System.Diagnostics.Tracing.EventSource.Initialize(System.Guid eventSourceGuid, string eventSourceName, string[] traits) Line 1458	C#
System.Private.CoreLib.dll!System.Diagnostics.Tracing.EventSource.EventSource(System.Diagnostics.Tracing.EventSourceSettings settings, string[] traits) Line 598	C#
System.Private.CoreLib.dll!System.Diagnostics.Tracing.EventSource.EventSource(System.Diagnostics.Tracing.EventSourceSettings settings) Line 569	C#
System.Private.CoreLib.dll!System.Diagnostics.Tracing.EventSource.EventSource(bool throwOnEventWriteErrors) Line 563	C#
eventlistener.exe!Tracing.Tests.SimpleEventSource.SimpleEventSource() Line 15	C#
eventlistener.exe!Tracing.Tests.EventPipeSmoke.Main(string[] args) Line 61	C#

This call path is reached because the EventListener enables events on the EventSource by calling SendCommand(Update). This command is deferred and processed in EventSource.Initialize, and before processing any command the EventSource checks to see if it needs to generate a manifest. We didn't hit this issue in previous tests because those tests directly called EventSource.Write without creating a derived EventSource type and declaring a new member on it.

Comparing ManifestBuilder to the encodings in SimpleTypeInfos, it appears that both of the types we encode as multiple fields would fail: Nullable and DateTimeOffset.

For Nullable, looking at other examples I assume we'd want a template such as:

<struct name=" [argument_name] " count="1" >
    <data name="HasValue" inType="win:Boolean" />
    <data name="Value" inType=" [GetTypeName(T)] " />
</struct>

[jbhensley]

I've only had a chance to look at this briefly, but the existing check for type.IsEnum reminds me that Nullable<T> supports enumerations. At first, the fix for ManifestBuilder.AddEventParameter looks fairly simple. Something like:

// support for nullable types
var baseType = Nullable.GetUnderlyingType(type);
if (baseType != null)
{
	templates.Append("   <struct name=\"").Append(name).Append("\" count=\"1\">").AppendLine();
	templates.Append("      <data name=\"HasValue\" inType=\"win:Boolean\"/>").AppendLine();
	templates.Append("      <data name=\"Value\" inType=\"").Append(GetTypeName(baseType)).Append("\"/>").AppendLine();
	templates.Append("   </struct>").AppendLine();
}
else   // not a nullable type
{   
	templates.Append("   <data name=\"").Append(name).Append("\" inType=\"").Append(GetTypeName(type)).Append("\"");

	// TODO: for 'byte*' types it assumes the user provided length is named using the same naming convention
	//       as for 'byte[]' args (blob_arg_name + "Size")
	if ((type.IsArray || type.IsPointer) && type.GetElementType() == typeof(byte))
	{
		// add "length" attribute to the "blob" field in the template (referencing the field added above)
		templates.Append(" length=\"").Append(name).Append("Size\"");
	}
	// ETW does not support 64-bit value maps, so we don't specify these as ETW maps
	if (type.IsEnum && Enum.GetUnderlyingType(type) != typeof(ulong) && Enum.GetUnderlyingType(type) != typeof(long))
	{
		templates.Append(" map=\"").Append(type.Name).Append("\"");
		if (mapsTab == null)
			mapsTab = new Dictionary<string, Type>();
		if (!mapsTab.ContainsKey(type.Name))
			mapsTab.Add(type.Name, type);        // Remember that we need to dump the type enumeration  
	}

	templates.Append("/>").AppendLine();
}

But you can create a method signature like this:

public enum TestEnum
{
	val1,val2,val3
}

public class TestEventSource : EventSource
{
	[Event(1)]
	public void Method1(int? nullableIntArg, int intArg, byte[] byteArrayArg, TestEnum? enumArg) { }
}

The enum map would need to be included as well, I assume on the Value element of the struct.

EDIT: baseType does return True for IsEnum

[jbhensley]

Given the above, the nullable case would probably look more like this:

// support for nullable types
var baseType = Nullable.GetUnderlyingType(type);
if (baseType != null)
{
	templates.Append("   <struct name=\"").Append(name).Append("\" count=\"1\">").AppendLine();
	templates.Append("      <data name=\"HasValue\" inType=\"win:Boolean\"/>").AppendLine();
	templates.Append("   ");
	AddEventParameter(baseType, "Value"); // since this can be a enumeration, it is eaiser to just call back in to the method with baseType
	templates.Append("   </struct>").AppendLine();
}

[noahfalk]

Chatting with @vancem he let me know that this ManifestBuilder functionality is largely a back-compat artifact for correct interoperation with legacy win7 and win8 era ETW readers. For any new usage he recommends customers create EventSource with the Self describing events. In this post he has a section describing that "Rich Data Only Supported in Self-Describing ETW" which corresponds to the EventSourceSettings.EtwSelfDescribingEventFormat flag in the constructor.

So my bad for not realizing that earlier. Perhaps the correct fix is to update the exception message, and perhaps check the docs to ensure we are pointing people in the right direction. For example the exception message could say "Nullable'1 is only supported when EventSource is using SelfDescribingEventFormat"

Technically we could support Nullable on the manifest format too, but I suspect we would run into other cases where SelfDescribing supports serializing a complex type and it is impossible to get Manifest mode to have parity. Its not clear it is worthwhile going part way down that path and then getting stuck.

[vancem]

Note that that bottom line here is that from a user perspective the fix is trivial. If you wish to use complex types (that is something besides primitives, strings and DateTime), then you need to use the EtwSelfDescribingFormat flag when creating the EventSouce. This is very easy. In your TestEventSource class simpy add

private TestEventSource() : base(EventSourceSettings.EtwSelfDescribingEventFormat) { }

Things should work from there. We recommend people do this in general. We would have made this the default, except is is a breaking change for people using EventSource in manifest based scenarios.

Certainly improving the documetation and error messages woudl help, but the bottom line is it is easy for you to get unblocked.

[jbhensley]

This certainly seemed like more than just some kind of bug, so unimplemented functionality totally makes sense. I'm going to remove the commented-out tests we added for this under PR dotnet/corefx#32777 and maybe leave a comment behind to the effect of what you have described here.

[dotnet-policy-service]

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

[dotnet-policy-service]

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.