-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace timestamp with time & Add document for GraphQL API connRawEvents
#918
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #918 +/- ##
==========================================
+ Coverage 77.17% 77.27% +0.09%
==========================================
Files 32 32
Lines 25723 25651 -72
==========================================
- Hits 19851 19821 -30
+ Misses 5872 5830 -42 ☔ View full report in Codecov by Sentry. |
src/graphql/network.rs
Outdated
timestamp: DateTime<Utc>, | ||
/// Source IP, or source IP address in long format. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Source IP address
only would suffice without Source IP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does long format
means?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is now modified to Source IP address.
. Long format was what I got from consultation with ChatGPT. I don't think we need it anymore.
src/graphql/network.rs
Outdated
orig_addr: String, | ||
/// Source port, or source port number in long format. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Source port number
would suffice without Source port
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
1a5dd56
to
d5f4dad
Compare
connRawEvents
connRawEvents
src/graphql/network/tests.rs
Outdated
@@ -3217,7 +3217,7 @@ async fn union() { | |||
} | |||
}"#; | |||
let res = schema.execute(query).await; | |||
assert_eq!(res.data.to_string(), "{networkRawEvents: {edges: [{node: {timestamp: \"2019-12-31T23:59:59+00:00\", __typename: \"BootpRawEvent\"}}, {node: {timestamp: \"2020-01-01T00:00:01+00:00\", __typename: \"SshRawEvent\"}}, {node: {timestamp: \"2020-01-01T00:00:05+00:00\", __typename: \"SmtpRawEvent\"}}, {node: {timestamp: \"2020-01-01T00:01:01+00:00\", __typename: \"ConnRawEvent\"}}, {node: {timestamp: \"2020-01-05T00:01:01+00:00\", __typename: \"RdpRawEvent\"}}, {node: {timestamp: \"2020-01-05T06:05:00+00:00\", __typename: \"DceRpcRawEvent\"}}, {node: {timestamp: \"2020-06-01T00:01:01+00:00\", __typename: \"HttpRawEvent\"}}, {node: {timestamp: \"2021-01-01T00:01:01+00:00\", __typename: \"DnsRawEvent\"}}, {node: {timestamp: \"2022-01-05T00:01:01+00:00\", __typename: \"NtlmRawEvent\"}}, {node: {timestamp: \"2023-01-05T00:01:01+00:00\", __typename: \"KerberosRawEvent\"}}, {node: {timestamp: \"2023-01-05T12:12:00+00:00\", __typename: \"FtpRawEvent\"}}, {node: {timestamp: \"2023-01-05T12:12:00+00:00\", __typename: \"MqttRawEvent\"}}, {node: {timestamp: \"2023-01-06T11:11:00+00:00\", __typename: \"TlsRawEvent\"}}, {node: {timestamp: \"2023-01-06T12:12:00+00:00\", __typename: \"LdapRawEvent\"}}, {node: {timestamp: \"2023-01-06T12:12:10+00:00\", __typename: \"SmbRawEvent\"}}, {node: {timestamp: \"2023-01-06T12:13:00+00:00\", __typename: \"NfsRawEvent\"}}, {node: {timestamp: \"2023-01-06T12:13:10+00:00\", __typename: \"DhcpRawEvent\"}}]}}"); | |||
assert_eq!(res.data.to_string(), "{networkRawEvents: {edges: [{node: {timestamp: \"2019-12-31T23:59:59+00:00\", __typename: \"BootpRawEvent\"}}, {node: {timestamp: \"2020-01-01T00:00:01+00:00\", __typename: \"SshRawEvent\"}}, {node: {timestamp: \"2020-01-01T00:00:05+00:00\", __typename: \"SmtpRawEvent\"}}, {node: {time: \"2020-01-01T00:01:01+00:00\", __typename: \"ConnRawEvent\"}}, {node: {timestamp: \"2020-01-05T00:01:01+00:00\", __typename: \"RdpRawEvent\"}}, {node: {timestamp: \"2020-01-05T06:05:00+00:00\", __typename: \"DceRpcRawEvent\"}}, {node: {timestamp: \"2020-06-01T00:01:01+00:00\", __typename: \"HttpRawEvent\"}}, {node: {timestamp: \"2021-01-01T00:01:01+00:00\", __typename: \"DnsRawEvent\"}}, {node: {timestamp: \"2022-01-05T00:01:01+00:00\", __typename: \"NtlmRawEvent\"}}, {node: {timestamp: \"2023-01-05T00:01:01+00:00\", __typename: \"KerberosRawEvent\"}}, {node: {timestamp: \"2023-01-05T12:12:00+00:00\", __typename: \"FtpRawEvent\"}}, {node: {timestamp: \"2023-01-05T12:12:00+00:00\", __typename: \"MqttRawEvent\"}}, {node: {timestamp: \"2023-01-06T11:11:00+00:00\", __typename: \"TlsRawEvent\"}}, {node: {timestamp: \"2023-01-06T12:12:00+00:00\", __typename: \"LdapRawEvent\"}}, {node: {timestamp: \"2023-01-06T12:12:10+00:00\", __typename: \"SmbRawEvent\"}}, {node: {timestamp: \"2023-01-06T12:13:00+00:00\", __typename: \"NfsRawEvent\"}}, {node: {timestamp: \"2023-01-06T12:13:10+00:00\", __typename: \"DhcpRawEvent\"}}]}}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
timestamp
here does not need to change to time
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{node: {timestamp: \"2020-01-01T00:01:01+00:00\", __typename: \"ConnRawEvent\"}}
is changed to
{node: {time: \"2020-01-01T00:01:01+00:00\", __typename: \"ConnRawEvent\"}}
The change of timestamp
to time
occurs here partially only for ConnRawEvent
, not for other events. It is because networkRawEvents
is a GraphQL query that returns an Union type, and in this case ConnRawEvent
is one of the types included in that Union type response. Since this PR targets to change ConnRawEvent
, only above part of code is modified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, do you think it is better to modify timestamp
to time
in all event protocols all at once, in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once we decided to changed timestamp
to time
, I think it would be better to complete it all at once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understood. I created an issue regarding timestamp-to-time as #922, and this PR has been updated to resolve that issue together at once.
678d34e
to
1c796cc
Compare
connRawEvents
connRawEvents
1c796cc
to
d107827
Compare
d107827
to
ae203ef
Compare
ae203ef
to
96fb059
Compare
@sehkone 시간 괜찮으시다면 리뷰 부탁드릴 수 있을까요? 최근 push 들은 단순 rebase들 입니다. |
Close: #917
Close: #922
Below was a description in PR draft stage:
Hi @sehkone, I’ve created this draft PR to ask for your confirmation if I’m heading in the right direction. So far, I’ve only worked on the
connRawEvents
GraphQL API as a single case example. If the approach aligns with your expectations, I can proceed to apply the same methodology to the remaining GraphQL APIs. Let me know your thoughts.GraphQL Playground looks like below: