{"id":5725,"date":"2020-02-03T15:00:13","date_gmt":"2020-02-03T15:00:13","guid":{"rendered":"http:\/\/writeasync.net\/?p=5725"},"modified":"2020-02-02T23:01:04","modified_gmt":"2020-02-02T23:01:04","slug":"lets-do-dhcp-more-diagnostic-events","status":"publish","type":"post","link":"http:\/\/writeasync.net\/?p=5725","title":{"rendered":"Let’s do DHCP: more diagnostic events"},"content":{"rendered":"

While thinking about how to combine latency measurement<\/a> with diagnostic events in the DHCP server<\/a>, I noticed a particular gap in the design. Right now the event interfaces look like this:<\/p>\n

\r\npublic interface ISocketEvents\r\n{\r\n    public void SendStart(SocketId id, int bufferSize, IPEndpointV4 endpoint)\r\n    {\r\n        \/\/ . . .\r\n    }\r\n \r\n    public void SendEnd(SocketId id, bool succeeded, Exception exception)\r\n    {\r\n        \/\/ . . .\r\n    }\r\n \r\n    \/\/ . . . etc.\r\n}\r\n<\/pre>\n
However, if we want to incorporate latency metrics, we would need to somehow initialize a TimePoint<\/code> in SendStart<\/code> and use it within SendEnd<\/code> to calculate the total duration. There are various fancy tricks we could employ in making this work but they would be just that — fancy tricks. Where possible, we should dispense with the magic and try just passing parameters<\/a>. What might that look like in this case?<\/p>\n
We can express the requirement as follows: we need the ability to create something<\/em> in XyzStart<\/code> and use that something<\/em> in XyzEnd<\/code>. It seems that the easiest way to proceed would be defining a state parameter:<\/p>\n
\r\npublic interface ISocketEvents<TState>\r\n{\r\n    public TState SendStart(SocketId id, int bufferSize, IPEndpointV4 endpoint)\r\n    {\r\n        \/\/ . . . return a TState instance here\r\n    }\r\n \r\n    public void SendEnd(SocketId id, bool succeeded, Exception exception, TState state)\r\n    {\r\n        \/\/ . . . use the TState instance here\r\n    }\r\n \r\n    \/\/ . . . etc.\r\n}\r\n<\/pre>\n
Now, tracking latency would be trivial:<\/p>\n
\r\npublic class SocketEventsWithLatency : ISocketEvents<TimePoint>\r\n{\r\n    public TimePoint SendStart(SocketId id, int bufferSize, IPEndpointV4 endpoint)\r\n    {\r\n        TimePoint start = TimePoint.Now();\r\n        \/\/ . . .\r\n        return start;\r\n    }\r\n \r\n    public void SendEnd(SocketId id, bool succeeded, Exception exception, TimePoint state)\r\n    {\r\n        \/\/ . . .\r\n        TimeSpan latency = TimePoint.Now() - state;\r\n        \/\/ send the latency value to some reporting system . . .\r\n    }\r\n \r\n    \/\/ . . . etc.\r\n}\r\n<\/pre>\n
This design is simple and flexible. Perhaps best of all, it is optional. If you can get by without a state parameter, then you can use the original non-generic interfaces.<\/p>\n
There is still one more thing bothering me about this design. Let’s take a look at the SendEnd<\/code> method:<\/p>\n
\r\n    public void SendEnd(SocketId id, bool succeeded, Exception exception, TState state)\r\n    {\r\n        \/\/ . . .\r\n    }\r\n<\/pre>\n
It might be hard to see the problem. But pay attention to the bool\/Exception<\/code> parameters. These are related items but they are passed separately — not just here, but in every<\/strong> XyzEnd<\/code> method. We have a data clump<\/a> which is begging to be extracted into a parameter object<\/a>. In this case, we could make a simple struct with nicely readable named constructors<\/a>:<\/p>\n
\r\n    public readonly struct OperationStatus\r\n    {\r\n        private OperationStatus(bool succeeded, Exception exception)\r\n        {\r\n            this.Succeeded = succeeded;\r\n            this.Exception = exception;\r\n        }\r\n\r\n        public bool Succeeded { get; }\r\n\r\n        public Exception Exception { get; }\r\n\r\n        public static OperationStatus Success() =>\r\n            new OperationStatus(true, null);\r\n\r\n        public static OperationStatus Failure(Exception exception) =>\r\n            new OperationStatus(false, exception);\r\n    }\r\n<\/pre>\n
It doesn’t look like much but even this small change does a lot to reveal the intention<\/a> of the calling code at a glance:<\/p>\n
\r\n            public async Task SendAsync(ReadOnlyMemory<byte> buffer, IPEndpointV4 endpoint)\r\n            {\r\n                Guid activityId = ActivityScope.CurrentId;\r\n                TState state = this.socketEvents.SendStart(this.id, buffer.Length, endpoint);\r\n                try\r\n                {\r\n                    await this.inner.SendAsync(buffer, endpoint);\r\n                    this.OnEndSend(activityId, OperationStatus.Success(), state);\r\n                }\r\n                catch (Exception e)\r\n                {\r\n                    this.OnEndSend(activityId, OperationStatus.Failure(e), state);\r\n                    throw;\r\n                }\r\n            }\r\n<\/pre>\n
For reference, this is what it looked like before:<\/p>\n
\r\n            public async Task SendAsync(ReadOnlyMemory<byte> buffer, IPEndpointV4 endpoint)\r\n            {\r\n                Guid activityId = ActivityScope.CurrentId;\r\n                TState state = this.socketEvents.SendStart(this.id, buffer.Length, endpoint);\r\n                try\r\n                {\r\n                    await this.inner.SendAsync(buffer, endpoint);\r\n                    this.OnEndSend(activityId, null, state);\r\n                }\r\n                catch (Exception e)\r\n                {\r\n                    this.OnEndSend(activityId, e, state);\r\n                    throw;\r\n                }\r\n            }\r\n<\/pre>\n
The structure is the same and it is still readable, but the success and failure conditions are somewhat more hidden. If nothing else, a pattern that explicitly uses the literal terms “Success” and “Failure” in its public interface ought to be harder to misuse.<\/p>\n
If you want the full details of this design and refactoring journey, check out the DhcpServer commits on GitHub<\/a>.<\/p>\n","protected":false},"excerpt":{"rendered":"
While thinking about how to combine latency measurement with diagnostic events in the DHCP server, I noticed a particular gap in the design. Right now the event interfaces look like this: public interface ISocketEvents { public void SendStart(SocketId id, int… <\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[91,71],"tags":[],"class_list":["post-5725","post","type-post","status-publish","format-standard","hentry","category-design","category-diagnostics"],"_links":{"self":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/posts\/5725","targetHints":{"allow":["GET"]}}],"collection":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/posts"}],"about":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Fcomments&post=5725"}],"version-history":[{"count":4,"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/posts\/5725\/revisions"}],"predecessor-version":[{"id":5729,"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/posts\/5725\/revisions\/5729"}],"wp:attachment":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Fmedia&parent=5725"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Fcategories&post=5725"},{"taxonomy":"post_tag","embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Ftags&post=5725"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}