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

[andr] Augment http logs with extra fields for gql requests #141

Merged
merged 7 commits into from
Dec 9, 2024

Conversation

murki
Copy link
Contributor

@murki murki commented Dec 6, 2024

Adds 3 fields (_operation_id, _operation_name, _operation_type) on top of all existing ones, plus using a diff name "_span_name": "_graphql",

http REST response:

    "fields": {
      "_app_version_code": "66",
      "_dns_resolution_duration_ms": "51",
      "_duration_ms": "171",
      "_host": "httpbin.org",
      "_locale": "en_US",
      "_method": "POST",
      "_path": "/post",
      "_path_template": "/post",
      "_request_body_bytes_sent_count": "11",
      "_request_headers_bytes_count": "113",
      "_response_body_bytes_received_count": "371",
      "_response_headers_bytes_count": "187",
      "_result": "success",
      "_span_id": "720dd844-67f0-40ce-a261-0b81a92789be",
      "_span_name": "_http",
      "_span_type": "end",
      "_status_code": "200",
      "app_version": "1.0",
      "carrier": "T-Mobile",
      "foreground": "1",
      "model": "sdk_gphone64_arm64",
      "network_type": "wlan",
      "os": "Android",
      "os_version": "14",
      "radio_type": "forbidden",
      "user_id": "e0d66ea4-1dfc-4156-b035-64a51395437b"
    },

http GraphQL response:

  "fields": {
      "_app_version_code": "66",
      "_dns_resolution_duration_ms": "29",
      "_duration_ms": "158",
      "_host": "apollo-fullstack-tutorial.herokuapp.com",
      "_locale": "en_US",
      "_method": "POST",
      "_operation_id": "07c30d89e22695b09840bcd4b83ce985db0a8b0950de9d6543be5f2c470ca2fc",
      "_operation_name": "LaunchList",
      "_operation_type": "query",
      "_path": "/graphql",
      "_path_template": "/graphql",
      "_request_body_bytes_sent_count": "133",
      "_request_headers_bytes_count": "628",
      "_response_body_bytes_received_count": "1648",
      "_response_headers_bytes_count": "769",
      "_result": "success",
      "_span_id": "5b29807b-5870-4268-b09c-7649d2cca8de",
      "_span_name": "_graphql",
      "_span_type": "end",
      "_status_code": "200",
      "app_version": "1.0",
      "carrier": "T-Mobile",
      "foreground": "1",
      "model": "sdk_gphone64_arm64",
      "network_type": "wlan",
      "os": "Android",
      "os_version": "14",
      "radio_type": "forbidden",
      "user_id": "e0d66ea4-1dfc-4156-b035-64a51395437b"
    },

@murki murki requested a review from snowp December 7, 2024 21:32
@murki murki changed the title Augment http logs with extra fields for gql requests [andr] Augment http logs with extra fields for gql requests Dec 7, 2024
.serverUrl("https://apollo-fullstack-tutorial.herokuapp.com/graphql")
.addInterceptor(CaptureApollo3Interceptor())
.okHttpClient(okHttpClient)
.addInterceptor(CaptureApolloInterceptor())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the extra integration point the customers would need to add. Note that this only works with okhttpclients that use our CaptureOkHttpEventListenerFactory

Comment on lines +34 to +37
// TODO(murki): Augment request logs with
// request.executionContext[CustomScalarAdapters]?.let {
// addHttpHeader("x-capture-span-gql-field-operation-variables", request.operation.variables(it).valueMap.toString())
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

? Is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's questionable whether gql variables https://graphql.org/learn/queries/#variables could contain PII, plus they're generally pretty big in size. Since we don't currently show it in our default dashboards I think we can ship this version without including them just yet

@murki murki merged commit 12bf4f0 into main Dec 9, 2024
15 checks passed
@murki murki deleted the murki/BIT-4116-2 branch December 9, 2024 14:59
@github-actions github-actions bot locked and limited conversation to collaborators Dec 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants