Skip to content
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

Overhaul Reporting for Data Science #295

Merged
merged 12 commits into from
Mar 29, 2023
Merged

Overhaul Reporting for Data Science #295

merged 12 commits into from
Mar 29, 2023

Conversation

Theodus
Copy link
Member

@Theodus Theodus commented Mar 28, 2023

I have made multiple attempts in the past to clean this up, which all failed to meaningfully improve the situation. The primary goal of this is to track information about queries as it becomes available in the code path, and moving the cumbersome backwards compatibility concerns somewhere else. This approach adds a custom layer to the tracing framework that collects events from special targets to eventually assemble the logs and Kafka messages for the data science team. It's a bit rough now. But I think it's finally a good starting point to facilitating the changing needs for standardization around gateway reporting.

This also adds subscription query Kafka messages.

@Theodus Theodus force-pushed the theodus/reporting branch 13 times, most recently from f41988b to 56bb034 Compare March 29, 2023 15:29
@Theodus Theodus force-pushed the theodus/reporting branch from 56bb034 to 83c9dd0 Compare March 29, 2023 15:29
@Theodus Theodus force-pushed the theodus/reporting branch from 8205a5f to 4627f97 Compare March 29, 2023 17:13
@Theodus Theodus marked this pull request as ready for review March 29, 2023 17:20
@Theodus Theodus requested a review from cmwhited March 29, 2023 17:20
Copy link
Contributor

@cmwhited cmwhited left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I like this layer approach. Super cool.

message GatewaySubscriptionQueryResult {
// Set to the value of the CF-Ray header, otherwise a generated UUID
string query_id = 1;
uint32 status_code = 2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just want to make sure, in the initial notion, this was an enum. I take it you decided against that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't since I didn't consider using an enum, but that's probably better. Fixed, updated Notion doc & reply below.


package kafka;

message GatewaySubscriptionQueryResult {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you send me the updated rust output from this .proto so I can include it in the graph-subscriptions-api? The fields look the same, but the order is different.

Copy link
Member Author

@Theodus Theodus Mar 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct GatewaySubscriptionQueryResult {
    /// Set to the value of the CF-Ray header, otherwise a generated UUID
    #[prost(string, tag = "1")]
    pub query_id: ::prost::alloc::string::String,
    #[prost(enumeration = "StatusCode", tag = "2")]
    pub status_code: i32,
    #[prost(string, tag = "3")]
    pub status_message: ::prost::alloc::string::String,
    #[prost(uint32, tag = "4")]
    pub response_time_ms: u32,
    /// `user` field from ticket payload, 0x-prefixed hex
    #[prost(string, tag = "5")]
    pub ticket_user: ::prost::alloc::string::String,
    /// `signer` field from ticket payload, 0x-prefixed hex
    #[prost(string, tag = "6")]
    pub ticket_signer: ::prost::alloc::string::String,
    /// `name` field from ticket payload
    #[prost(string, optional, tag = "7")]
    pub ticket_name: ::core::option::Option<::prost::alloc::string::String>,
    /// Subgraph Deployment ID, CIDv0 ("Qm" hash)
    #[prost(string, optional, tag = "8")]
    pub deployment: ::core::option::Option<::prost::alloc::string::String>,
    /// Chain name indexed by subgraph deployment
    #[prost(string, optional, tag = "9")]
    pub subgraph_chain: ::core::option::Option<::prost::alloc::string::String>,
    #[prost(uint32, optional, tag = "10")]
    pub query_count: ::core::option::Option<u32>,
    #[prost(float, optional, tag = "11")]
    pub query_budget: ::core::option::Option<f32>,
    #[prost(float, optional, tag = "12")]
    pub indexer_fees: ::core::option::Option<f32>,
}
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)]
#[repr(i32)]
pub enum StatusCode {
    Success = 0,
    InternalError = 1,
    UserError = 2,
    NotFound = 3,
}
impl StatusCode {
    /// String value of the enum field names used in the ProtoBuf definition.
    ///
    /// The values are not transformed in any way and thus are considered stable
    /// (if the ProtoBuf definition does not change) and safe for programmatic use.
    pub fn as_str_name(&self) -> &'static str {
        match self {
            StatusCode::Success => "SUCCESS",
            StatusCode::InternalError => "INTERNAL_ERROR",
            StatusCode::UserError => "USER_ERROR",
            StatusCode::NotFound => "NOT_FOUND",
        }
    }
    /// Creates an enum from field names used in the ProtoBuf definition.
    pub fn from_str_name(value: &str) -> ::core::option::Option<Self> {
        match value {
            "SUCCESS" => Some(Self::Success),
            "INTERNAL_ERROR" => Some(Self::InternalError),
            "USER_ERROR" => Some(Self::UserError),
            "NOT_FOUND" => Some(Self::NotFound),
            _ => None,
        }
    }
}

@Theodus Theodus merged commit 73fff9f into main Mar 29, 2023
@Theodus Theodus deleted the theodus/reporting branch March 29, 2023 19:54
@Theodus Theodus mentioned this pull request Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants