Add document for GraphQL API `dnsRawEvents` #984

danbi2990 · 2025-01-21T06:33:09Z

Close: #964

codecov · 2025-01-21T06:40:46Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.48%. Comparing base (5b643fa) to head (49d7893).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #984   +/-   ##
=======================================
  Coverage   77.48%   77.48%           
=======================================
  Files          32       32           
  Lines       25799    25799           
=======================================
  Hits        19990    19990           
  Misses       5809     5809

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/graphql/network.rs

src/graphql/client/schema/schema.graphql

sehkone · 2025-01-22T23:23:31Z

src/graphql/network.rs

    proto: u8,
+    /// End time in nanoseconds. `time` + `rtt` (Default).


(Default) looks confusing because it implies there might be other cases. I assume this last_time is not parsed but calculated from time + rtt by the software, and (Default) doesn't seem to describe it accurately. I also think the content from time might not be necessary. @syncpark, could you confirm this?

@syncpark
Could you please review this comment? It suggests the following change.

- End time in nanoseconds. `time` + `rtt` (Default). + End time in nanoseconds.

@danbi2990 Let's just make this part the same as we did in other events, such as https://github.com/aicers/giganto/pull/1021/files#diff-353689134b6a76113df88a614d4b93d4a96a3bcacf8bc163837255dbec1c2286R720-R722

sehkone · 2025-01-22T23:33:17Z

src/graphql/network.rs

+    /// Query type.
+    ///
+    /// - 1: Internet class
+    /// - 2: CSNET class
+    /// - 3: CHAOS class
+    /// - 4: Hesiod class


These values belong to qclass, not qtype. In the document where attributes are described, content other than my suggested titles has not been verified. Therefore, we should not copy it unless it has been strictly verified.

@sophie-cluml,

For now, I think it would be better to include just the titles, not the additional content written in the document. If such additional content is included in another context, we should either remove it or strictly verify it.

Applied the suggested adjustment.

sehkone · 2025-01-22T23:43:48Z

src/graphql/network.rs

+    /// Response code. Range: 0-11, 16-23.
+    ///
+    /// |RCODE|Name|Description|
+    /// |-|-|-|
+    /// |0|NoError|No Error|
+    /// |1|FormErr|Format Error|
+    /// |2|ServFail|Server Failure|
+    /// |3|NXDomain|Non-Existent Domain|
+    /// |4|NotImp|Not Implemented|
+    /// |5|Refused|Query Refused|
+    /// |6|YXDomain|Name Exists when it should not|
+    /// |7|YXRRSet|RR Set Exists when it should not|
+    /// |8|NXRRSet|RR Set that should exist does not|
+    /// |9|NotAuth|Server Not Authoritative for zone|
+    /// |9|NotAuth|Not Authorized|
+    /// |10|NotZone|Name not contained in zone|
+    /// |11|DSOTYPENI|DSO-TYPE Not Implemented|
+    /// |12-15|Unassigned||
+    /// |16|BADVERS|Bad OPT Version|
+    /// |16|BADSIG|TSIG Signature Failure|
+    /// |17|BADKEY|Key not recognized|
+    /// |18|BADTIME|Signature out of time window|
+    /// |19|BADMODE|Bad TKEY Mode|
+    /// |20|BADNAME|Duplicate key name|
+    /// |21|BADALG|Algorithm not supported|
+    /// |22|BADTRUNC|Bad Truncation|
+    /// |23|BADCOOKIE|Bad/missing Server Cookie|


I think only Response code would be better. Could you remove the others?

Applied the suggested adjustment.

sehkone · 2025-01-22T23:45:05Z

src/graphql/network.rs

    rcode: u16,
+    /// Authoritative answer flag.
+    ///
+    /// Only from DNS response message.


I think this line is unnecessary.

Applied the suggested adjustment.

sehkone · 2025-01-22T23:45:22Z

src/graphql/network.rs

    aa_flag: bool,
+    /// Truncated flag.
+    ///
+    /// Truncated flag of DNS query and response message.


So does this.

Applied the suggested adjustment.

sehkone · 2025-01-22T23:45:36Z

src/graphql/network.rs

    tc_flag: bool,
+    /// Recursion desired flag.
+    ///
+    /// Recursion desired flag of DNS query and response message.


Applied the suggested adjustment.

sehkone · 2025-01-22T23:45:51Z

src/graphql/network.rs

    rd_flag: bool,
+    /// Recursion available flag.
+    ///
+    /// Only from DNS response message.


This one as well.

Applied the suggested adjustment.

sehkone · 2025-01-22T23:48:04Z

src/graphql/network.rs

    rtt: StringNumberI64,
+    /// Query class. Range: 1-4.
+    ///
+    /// For more information, refer to [Domain Name System Parameters.](https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-4)


If we include a reference in one context, we should do the same in every context. Therefore, I think it would be better to remove it.

Applied the suggested adjustment.

sophie-cluml · 2025-02-19T06:07:22Z

src/graphql/client/schema/schema.graphql

  respPort: Int!
+
+  # Protocol number. TCP is 6, UDP is 17.


I think we can modify this part similarly to other events as well.

Updated the code like the others.

sophie-cluml · 2025-02-19T08:53:19Z

src/graphql/client/schema/schema.graphql

  raFlag: Boolean!
+
+  # Time to live


Suggested change

# Time to live

# Time to Live

as it was updated in the Notion.

Applied the suggested adjustment.

Close: #964

sophie-cluml · 2025-02-20T08:28:57Z

@sehkone I believe this PR is well aligned with our policy regarding the GraphQL API documentation, since we have already synced in other similar PRs. And I also believe the points that you have raised in previous comments are all resolved. So let me merge this PR. If you think we should not merge, I'd appreciate if you could let us know.

danbi2990 requested a review from sophie-cluml January 21, 2025 06:33

danbi2990 changed the title ~~Add document for DnsRawEvent~~ Add document for GraphQL API dnsRawEvents Jan 21, 2025

sophie-cluml reviewed Jan 21, 2025

View reviewed changes

danbi2990 force-pushed the jake/add-graphql-doc-DNS branch 2 times, most recently from 7a3e1b7 to 75d80b9 Compare January 21, 2025 08:30

sophie-cluml reviewed Jan 21, 2025

View reviewed changes

src/graphql/client/schema/schema.graphql Outdated Show resolved Hide resolved

danbi2990 force-pushed the jake/add-graphql-doc-DNS branch 2 times, most recently from 5a3f446 to 9c6ad30 Compare January 22, 2025 06:01

sophie-cluml reviewed Jan 22, 2025

View reviewed changes

src/graphql/client/schema/schema.graphql Outdated Show resolved Hide resolved

danbi2990 force-pushed the jake/add-graphql-doc-DNS branch from 9c6ad30 to 5c943f5 Compare January 22, 2025 08:00

sophie-cluml approved these changes Jan 22, 2025

View reviewed changes

sehkone reviewed Jan 22, 2025

View reviewed changes

danbi2990 force-pushed the jake/add-graphql-doc-DNS branch 6 times, most recently from 5f74fba to 73d9034 Compare February 6, 2025 04:29

danbi2990 force-pushed the jake/add-graphql-doc-DNS branch from 73d9034 to 96aea48 Compare February 11, 2025 04:28

sophie-cluml reviewed Feb 19, 2025

View reviewed changes

danbi2990 force-pushed the jake/add-graphql-doc-DNS branch from 96aea48 to a647475 Compare February 19, 2025 06:15

sophie-cluml reviewed Feb 19, 2025

View reviewed changes

danbi2990 force-pushed the jake/add-graphql-doc-DNS branch 2 times, most recently from b3422cb to 9a3433e Compare February 19, 2025 08:57

Add documentation for DnsRawEvent

49d7893

Close: #964

danbi2990 force-pushed the jake/add-graphql-doc-DNS branch from 9a3433e to 49d7893 Compare February 20, 2025 05:39

sophie-cluml approved these changes Feb 20, 2025

View reviewed changes

sophie-cluml merged commit 779fb61 into main Feb 21, 2025
10 checks passed

sophie-cluml deleted the jake/add-graphql-doc-DNS branch February 21, 2025 01:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add document for GraphQL API `dnsRawEvents` #984

Add document for GraphQL API `dnsRawEvents` #984

danbi2990 commented Jan 21, 2025

codecov bot commented Jan 21, 2025 •

edited

Loading

sehkone Jan 22, 2025

danbi2990 Feb 17, 2025

sophie-cluml Feb 19, 2025

sehkone Jan 22, 2025

sehkone Jan 22, 2025

danbi2990 Feb 3, 2025

sehkone Jan 22, 2025 •

edited

Loading

danbi2990 Feb 3, 2025

sehkone Jan 22, 2025

danbi2990 Feb 3, 2025

sehkone Jan 22, 2025

danbi2990 Feb 3, 2025

sehkone Jan 22, 2025

danbi2990 Feb 3, 2025

sehkone Jan 22, 2025

danbi2990 Feb 3, 2025

sehkone Jan 22, 2025

danbi2990 Feb 3, 2025

sophie-cluml Feb 19, 2025

danbi2990 Feb 19, 2025

sophie-cluml Feb 19, 2025

danbi2990 Feb 19, 2025

sophie-cluml commented Feb 20, 2025

		proto: u8,
		/// End time in nanoseconds. `time` + `rtt` (Default).

Add document for GraphQL API dnsRawEvents #984

Add document for GraphQL API dnsRawEvents #984

Conversation

danbi2990 commented Jan 21, 2025

codecov bot commented Jan 21, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sehkone Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sophie-cluml commented Feb 20, 2025

Add document for GraphQL API `dnsRawEvents` #984

Add document for GraphQL API `dnsRawEvents` #984

codecov bot commented Jan 21, 2025 •

edited

Loading

sehkone Jan 22, 2025 •

edited

Loading