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

feat: log graphql requests #526

Open
wants to merge 12 commits into
base: dev
Choose a base branch
from
Open

feat: log graphql requests #526

wants to merge 12 commits into from

Conversation

jsstevenson
Copy link
Contributor

@jsstevenson jsstevenson commented Oct 14, 2024

This PR grabs the query type and the declared variables and logs them.

A couple notes/TODO's

  • Aspirationally I wanted to see if we could be grabbing whether or not specific parameters get used under the Drug node, but it seems kind of complicated to figure that out (is there a reasonable way to tell if a query includes a drug node at all? I'm not sure)

@jsstevenson jsstevenson marked this pull request as ready for review October 21, 2024 20:27
require 'net/http'

class SubmitAnalyticsEvent < ApplicationJob
# TODO not entirely sure what goes here
Copy link
Contributor Author

Choose a reason for hiding this comment

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

^

{
searchTerms: names,
},
{ 'dgidb-client-name': 'dgidb-frontend' }
Copy link
Collaborator

Choose a reason for hiding this comment

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

You might want to double check that it works as expected, but it looks like (based on this example here) you can set an HTTP Header when your initialize the graphql client.

That might be better than stapling it into each query individually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@acoffman yeah, I was a little unsure of whether we wanted to set different headers for different queries and to filter out noisier or less informative ones. We should probably set a base one at client initialization and then maybe use an additional one for chosen queries if we go that route.

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