Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore!: route connection logs to new table#18340

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ethanndickson merged 1 commit intomainfromethan/reroute-connection-logs
Jul 15, 2025

Conversation

ethanndickson
Copy link
Member

@ethanndicksonethanndickson commentedJun 12, 2025
edited
Loading

Breaking Change (changelog note):

Connections to workspaces (via SSH, workspace apps, or browser port-forwarding) will no longer create entries in the audit log. Those events will now be included in the 'Connection Log'.
Please see the 'Connection Log' page in the dashboard, and the Connection Logdocumentation for details. Those with permission to view the Audit Log will also be able to view the Connection Log. The new Connection Log has the same licensing restrictions as the Audit Log, and requires a Premium Coder deployment.

Context

This is the first PR of a few for moving connection events out of the audit log, and into a new database table and web UI page called the 'Connection Log'.

This PR:

  • Creates the new table
  • Adds and tests queries for inserting and reading, including reading with an RBAC filter.
  • Implements the corresponding RBAC changes, such that anyone who can view the audit log can read from the table
  • Implements, under the enterprise package, aConnectionLogger abstraction to replace theAuditor abstraction for these logs. (No-op'd in AGPL, like theAuditor)
  • Routes SSH connection and Workspace App events into the newConnectionLogger
  • Updates all existing tests to check the values of theConnectionLogger instead of theAuditor.

Future PRs:

  • Add filtering to the query
  • Add an enterprise endpoint to query the new table
  • Write a query to delete old events from the audit log, call it from dbpurge.
  • Implement a table in the Web UI for viewing connection logs.

Note

The PRs in this stack obviously won't be (completely) atomic. Whilst they'll each pass CI, the stack is designed to be merged all at once. I'm splitting them up for the sake of those reviewing, and so changes can be reviewed as early as possible. Despite this, it's really hard to make this PR any smaller than it already is. I'll be keeping it in draft until it's actually ready to merge.

Copilot

This comment was marked as resolved.

@ethanndickson
Copy link
MemberAuthor

Requesting a draft review just to confirm we're happy with the DB schema, the RBAC setup, and the overall direction.

Copy link
Member

@mafredrimafredri left a comment

Choose a reason for hiding this comment

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

Nice work! I think this looks great in general, but the upsert part of the logic here doesn't fully make sense to me (see related comment). Could you elaborate on the intent?

@github-actionsgithub-actionsbot added the staleThis issue is like stale bread. labelJun 24, 2025
@ethanndicksonethanndickson removed the staleThis issue is like stale bread. labelJun 24, 2025
@ethanndicksonethanndicksonforce-pushed theethan/reroute-connection-logs branch 2 times, most recently from947ccd1 to5231befCompareJune 27, 2025 09:13
@ethanndicksonethanndicksonforce-pushed theethan/reroute-connection-logs branch frome4e1a04 to1fcd8d7CompareJune 30, 2025 12:03
@ethanndicksonethanndickson changed the titlechore: route connection logs to new tablechore!: route connection logs to new tableJul 2, 2025
@ethanndicksonethanndickson added the release/breakingThis label is applied to PRs to detect breaking changes as part of the release process labelJul 2, 2025
@ethanndicksonethanndickson marked this pull request as ready for reviewJuly 3, 2025 08:23
Copilot

This comment was marked as outdated.

Copy link
Member

@mafredrimafredri left a comment

Choose a reason for hiding this comment

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

Some minor nits and suggestions but largely looks alright to me, nice work! 👍🏻

Also, obligatory:./coderd/database/migrations/fix_migration_numbers.sh.

@ethanndicksonethanndicksonforce-pushed theethan/reroute-connection-logs branch 2 times, most recently fromf47a3f7 to0346609CompareJuly 10, 2025 10:28
Copy link
Contributor

@CopilotCopilotAI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a dedicatedconnection_logs table and replaces audit-based connection events with a newConnectionLogger abstraction, ensuring SSH and Workspace App connections are recorded in their own table and UI. Key changes include:

  • Schema and SQL migrations forconnection_logs, plus generated Go query methods and RBAC filters.
  • NewConnectionLogger abstraction in both enterprise and open core, replacing audit calls for connection events.
  • Updates across CLI, API server, agent API, and tests to route connection events throughConnectionLogger.

Reviewed Changes

Copilot reviewed 54 out of 54 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
enterprise/coderd/connectionlog/connectionlog.goImplementsBackend andConnectionLogger types
coderd/database/migrations/000349_connection_logs.up.sqlCreatesconnection_logs table and types
coderd/database/queries/connectionlogs.sqlSQL for reading and upserting connection logs
coderd/workspaceapps/db.goReplaces audit logic with connection logging
coderd/agentapi/connectionlog.goNew agent API endpoint for reporting connections
coderd/database/types.goAddsParseIP helper forinet type
coderd/database/queries.sql.goAdds generatedGetConnectionLogsOffset andUpsertConnectionLog methods
Comments suppressed due to low confidence (2)

coderd/workspaceapps/db.go:97

  • [nitpick] The variable namecommitAudit is misleading since this sets up connection logging, not audit logging. Consider renaming it tocommitConnLog orcommitConnectionLog for clarity.
aReq, commitAudit := p.connLogInitRequest(ctx, rw, r)

coderd/workspaceapps/db.go:394

  • [nitpick] The comment uses “connect log” where “connection log” would match the terminology used elsewhere. Consider updating for consistency.
// connLogInitRequest creates a new connection log session and connect log for the

@ethanndicksonethanndicksonforce-pushed theethan/reroute-connection-logs branch 3 times, most recently from7d13236 tobea35a3CompareJuly 15, 2025 03:33
@ethanndicksonethanndicksonforce-pushed theethan/reroute-connection-logs branch frombea35a3 to3164ea2CompareJuly 15, 2025 04:00
@ethanndicksonGraphite App
Copy link
MemberAuthor

ethanndickson commentedJul 15, 2025
edited
Loading

Merge activity

  • Jul 15, 4:35 AM UTC: A user started a stack merge that includes this pull request viaGraphite.
  • Jul 15, 4:36 AM UTC:@ethanndickson merged this pull request withGraphite.

@ethanndicksonethanndickson merged commit08e17a0 intomainJul 15, 2025
34 of 36 checks passed
@ethanndicksonethanndickson deleted the ethan/reroute-connection-logs branchJuly 15, 2025 04:36
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@mafredrimafredrimafredri left review comments

@EmyrkEmyrkEmyrk left review comments

Copilot code reviewCopilotCopilot left review comments

@deansheatherdeansheatherdeansheather approved these changes

Assignees

@ethanndicksonethanndickson

Labels
release/breakingThis label is applied to PRs to detect breaking changes as part of the release process
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@ethanndickson@mafredri@Emyrk@deansheather

[8]ページ先頭

©2009-2025 Movatter.jp