maskit commented on a change in pull request #6836:
URL: https://github.com/apache/trafficserver/pull/6836#discussion_r434916251
##########
File path: iocore/net/quic/QUICLogFrame.h
##########
@@ -0,0 +1,286 @@
+#pragma once
+
+#include <memory>
+#include <yaml-cpp/yaml.h>
+
+#include "QUICFrame.h"
+
+namespace QLog
+{
+class QLogFrame
+{
+public:
+ QLogFrame(QUICFrameType type) : _type(type) {}
+ virtual ~QLogFrame() {}
+
+ QUICFrameType
+ type() const
+ {
+ return this->_type;
+ }
+
+ // encode frame into YAML stype
+ virtual void encode(YAML::Node &node) = 0;
+
+protected:
+ QUICFrameType _type = QUICFrameType::UNKNOWN;
+};
+
+using QLogFrameUPtr = std::unique_ptr<QLogFrame>;
+
+//
+// convert QUICFrame to QLogFrame
+//
+class QLogFrameFactory
+{
+public:
+ // create QLogFrame
+ static QLogFrameUPtr create(QUICFrame &frame);
+};
+
+namespace Frame
+{
+ struct AckFrame : public QLogFrame {
+ AckFrame(QUICAckFrame &frame) : QLogFrame(frame.type())
Review comment:
I changed my mind a little. I read the qlog spec and found that the spec
have `raw` field that seems like to keep entire frame data (the both header and
payload). So we might want a way to keep all the data for people who want it.
In that case, using QUICFrame may make sense. However, the behavior should
be configurable if we support it. I think we don't want such huge logs in most
cases (we've never needed it for H2).
So, for the first step, I think current design is good enough. qlog spec is
still a draft yet, and I don't think we should spend much time to support the
minor use case. Supporting other events would be more valuable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
