Skip to content

Conversation

@ramonsmits
Copy link
Member

@ramonsmits ramonsmits commented Aug 14, 2023

Symptoms

Monitoring cannot process messages where the EnclosedMessageTypes value contains multiple values which results in excessive retries and results in rate limiting issues on some transports.

The received value is the value from NServiceBus.EnclosedMessageTypes which can contain multiple types.

This should resolve the issue that MonitoredEndpointMessageTypeParser.Parse receives the value with all types and results in the following log entry:

2023-08-14 10:19:48.5594|8|Warn|ServiceControl.Monitoring.Http.Diagrams.MonitoredEndpointMessageTypeParser|Error parsing message type: XXXX, YYY, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null;ZZZZ, YYY, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null.
System.IO.FileLoadException: The given assembly name or codebase was invalid. (Exception from HRESULT: 0x80131047)
   at System.Reflection.AssemblyName.nInit(RuntimeAssembly& assembly, Boolean forIntrospection, Boolean raiseResolveEvent)
   at System.Reflection.AssemblyName..ctor(String assemblyName)
   at ServiceControl.Monitoring.Http.Diagrams.MonitoredEndpointMessageTypeParser.Parse(String typeName) in /_/src/ServiceControl.Monitoring/Http/Diagrams/MonitoredEndpointMessageTypeParser.cs:line 31

Who's affected

Users that use ServiceControl Monitoring and that send polymorphic messages (messages that use inheritance).

Root cause

Monitoring cannot process messages when EnclosedMessageTypes contains multiple values

@ramonsmits ramonsmits added the Bug label Aug 14, 2023
@ramonsmits ramonsmits self-assigned this Aug 14, 2023
@ramonsmits ramonsmits changed the title The received value is the value from NServiceBus.EnclosedMessageTypes… Fix flooding the logs with "Error parsing message type:" for metrics from polymorphic messages Aug 14, 2023
@ramonsmits ramonsmits force-pushed the MonitoredEndpointMessageTypeParser branch from 873f655 to 850ff11 Compare August 14, 2023 15:54
@danielmarbach
Copy link
Contributor

I apologize for my inputs coming in with such big delays. I have a few absences this week. These are wild thoughts. I'm fairly new to this code.

The original stack trace showed the problem coming from MonitoredEndpointMessageTypeParser and now we have essentially multiple places where we have to incorporate the splitting logic. The parser itself already has to deal with endpoint message types and the controller can already deal with multiple message types

https://github.com/Particular/ServiceControl/blob/master/src/ServiceControl.Monitoring/Http/Diagrams/DiagramApiController.cs#L254-L258

I wonder if it would be better to consolidate things a bit to centralize the parsing and creation of EndpointMessageType and make the MonitoredEndpointMessageTypeParser rely on that type too. This would essentially make sure polymorphic message types are visualized as multiple message types. Polymorphic types are always separated by ; see https://github.com/Particular/NServiceBus/blob/f9a03dc930c20dc80b18034f824be51fc2adbacd/src/NServiceBus.Core/Unicast/Messages/MessageMetadata.cs#L62. The parser could take that into account. Core also seems to treat those polymorphic ones as individual ones in multiple places

https://github.com/Particular/NServiceBus/blob/master/src/NServiceBus.Core/Pipeline/Incoming/DeserializeMessageConnector.cs#L72-L97
https://github.com/Particular/NServiceBus/blob/master/src/NServiceBus.Core/Pipeline/Incoming/DeserializeMessageConnector.cs#L120-L126
https://github.com/Particular/NServiceBus/blob/master/src/NServiceBus.Core/Routing/UnicastPublishRouter.cs#L28-L30

@ramonsmits ramonsmits marked this pull request as ready for review September 5, 2023 15:20
@danielmarbach danielmarbach merged commit 4c6ec9d into master Sep 19, 2023
@danielmarbach danielmarbach deleted the MonitoredEndpointMessageTypeParser branch September 19, 2023 10:58
@ramonsmits ramonsmits added this to the 5.0.0 milestone Oct 18, 2023
ramonsmits added a commit that referenced this pull request Oct 18, 2023
…from polymorphic messages (#3661)

* The received value is the value from NServiceBus.EnclosedMessageTypes which can contain multiple types

* Modifed tests to use polymorphic messages

* No longer affected, removing validation

* Moved parsing to contructor of `EndpointMessageType`
@ramonsmits ramonsmits changed the title Fix flooding the logs with "Error parsing message type:" for metrics from polymorphic messages Monitoring cannot process messages where EnclosedMessageTypes contains multiple values Oct 25, 2023
@ramonsmits ramonsmits changed the title Monitoring cannot process messages where EnclosedMessageTypes contains multiple values Monitoring cannot process metrics for processed polymorphic messages Oct 25, 2023
@ramonsmits ramonsmits removed this from the 5.0.0 milestone Nov 30, 2023
@ramonsmits ramonsmits added this to the 5.0.0 milestone Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants