Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Make GraphQL logs an optional part of the audit logs. (take 2)#42550

Merged
vrto merged 6 commits into
mainfrom
mv/auditlog/graphql-logs-2
Oct 10, 2022
Merged

Make GraphQL logs an optional part of the audit logs. (take 2)#42550
vrto merged 6 commits into
mainfrom
mv/auditlog/graphql-logs-2

Conversation

@vrto

@vrto vrto commented Oct 5, 2022

Copy link
Copy Markdown
Contributor

Description

GraphQL logs will optionally be included in the audit log.

Further details in RFC 734.

Implementation details

This is another take on the work done in https://github.com/sourcegraph/sourcegraph/pull/41952

The audit logs are (optionally) recorded in the serveGraphQL function. This is a better place to host the logic as:

The previous approach with LOG_ALL_GRAPHQL_REQUESTS is left intact and scheduled for removal.

Enabling the audit log

  "log": {
    "auditLog": {
      "securityEvents": true,
      "graphQL": true,
      "gitserverAccess": false
    }
  }

Sample audit log

{
  "SeverityText": "INFO",
  "Timestamp": 1664960017940667000,
  "InstrumentationScope": "Handler",
  "Caller": "audit/audit.go:33",
  "Function": "github.com/sourcegraph/sourcegraph/internal/audit.Log",
  "Body": "request (sampling immunity token: a0282b84-f9cc-4e1c-86a6-49692514b9e8)",
  "Resource": {
    "service.name": "frontend",
    "service.version": "0.0.0+dev",
    "service.instance.id": "Michals-MacBook-Pro-2.local"
  },
  "Attributes": {
    "audit": {
      "auditId": "a0282b84-f9cc-4e1c-86a6-49692514b9e8",
      "entity": "GraphQL",
      "actor": {
        "actorUID": "1",
        "ip": "127.0.0.1",
        "X-Forwarded-For": "127.0.0.1, 127.0.0.1"
      }
    },
    "request": {
      "name": "LogEvents",
      "source": "browser",
      "variables": "{\"events\":[{\"argument\":\"true\",\"cohortID\":\"2022-09-19\",\"deviceID\":\"8cd45f87-f693-44a3-9bb0-ff86870ad26a\",\"event\":\"SiteAdminReposViewed\",\"eventID\":9,\"firstSourceURL\":\"https://sourcegraph.test:3443/sign-in?returnTo=%2Fsearch\",\"insertID\":\"60e14143-4508-4a5a-9d5f-9c57cb839928\",\"lastSourceURL\":\"https://sourcegraph.test:3443/sign-in?returnTo=%2Fsearch\",\"publicArgument\":null,\"referrer\":\"\",\"source\":\"WEB\",\"url\":\"https://sourcegraph.test:3443/site-admin/repositories\",\"userCookieID\":\"8cd45f87-f693-44a3-9bb0-ff86870ad26a\"}]}",
      "query": "\n    mutation LogEvents($events: [Event!]) {\n        logEvents(events: $events) {\n            alwaysNil\n        }\n    }\n"
    },
    "mutation": true,
    "successful": true
  }
}

Test plan

@cla-bot cla-bot Bot added the cla-signed label Oct 5, 2022
@vrto vrto requested review from a team, bobheadxi and jhchabran October 5, 2022 09:14
@vrto vrto mentioned this pull request Oct 5, 2022
1 task
@vrto vrto requested a review from rafax October 5, 2022 09:20
Comment thread cmd/frontend/internal/httpapi/graphql.go Outdated

@bobheadxi bobheadxi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nice, other than minor nits - thanks for the testing as well!

Comment thread cmd/frontend/graphqlbackend/graphqlbackend.go
Comment thread cmd/frontend/internal/httpapi/graphql.go Outdated
@vrto vrto merged commit 0b48a40 into main Oct 10, 2022
@vrto vrto deleted the mv/auditlog/graphql-logs-2 branch October 10, 2022 08:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants