Skip to content

do not log stream close error for EOF#37240

Merged
GavinFrazar merged 1 commit intomasterfrom
gavinfrazar/fix-session-streamer-error-logging
Jan 30, 2024
Merged

do not log stream close error for EOF#37240
GavinFrazar merged 1 commit intomasterfrom
gavinfrazar/fix-session-streamer-error-logging

Conversation

@GavinFrazar
Copy link
Copy Markdown
Contributor

@GavinFrazar GavinFrazar commented Jan 25, 2024

This PR eliminates a fake error log when closing file session uploads.

I noticed that every time I disconnected from a database, I would see this in the agent logs:

2024-01-24T15:29:46-08:00 DEBU [DB:SERVIC] Stop receiving from client. db:teleport-postgres-eastus-database-access-060a97ea-3a57-4218-9be5-dba3f19ff2b5 from:client id:432037ec-3dd4-46b1-964f-a2c28fa9352e postgres/engine.go:351
2024-01-24T15:29:46-08:00 DEBU [DB:SERVIC] Client done. db:teleport-postgres-eastus-database-access-060a97ea-3a57-4218-9be5-dba3f19ff2b5 error:[<nil>] id:432037ec-3dd4-46b1-964f-a2c28fa9352e postgres/engine.go:186
2024-01-24T15:29:46-08:00 DEBU [DB:SERVIC] Server connection closed. db:teleport-postgres-eastus-database-access-060a97ea-3a57-4218-9be5-dba3f19ff2b5 from:server id:432037ec-3dd4-46b1-964f-a2c28fa9352e postgres/engine.go:441
2024-01-24T15:29:46-08:00 DEBU [DB:SERVIC] Stopped parsing messages from server. db:teleport-postgres-eastus-database-access-060a97ea-3a57-4218-9be5-dba3f19ff2b5 from:server id:432037ec-3dd4-46b1-964f-a2c28fa9352e parsed_total:15 postgres/engine.go:434
2024-01-24T15:29:46-08:00 DEBU [DB:SERVIC] Stopped receiving from server. Transferred 1299 bytes. db:teleport-postgres-eastus-database-access-060a97ea-3a57-4218-9be5-dba3f19ff2b5 from:server id:432037ec-3dd4-46b1-964f-a2c28fa9352e postgres/engine.go:466
2024-01-24T15:29:46-08:00 INFO [AUDIT]     db.session.end cluster_name:mars.internal code:TDB01I db_name:postgres db_origin:cloud db_protocol:postgres db_service:teleport-postgres-eastus-database-access-060a97ea-3a57-4218-9be5-dba3f19ff2b5 db_type:azure db_uri:teleport-postgres.postgres.database.azure.com:5432 db_user:database-access-group ei:2 event:db.session.end private_key_policy:none sid:432037ec-3dd4-46b1-964f-a2c28fa9352e time:2024-01-24T23:29:46.947Z uid:e75146ce-5085-43a8-a7f2-aaec13e57aef user:martian user_kind:1 with_mfa:009ef81b-f0df-48fa-a853-fe441a1555c1 events/emitter.go:278
2024-01-24T15:29:51-08:00 DEBU [UPLOAD]    Scanned 1 uploads, started 1, found 0 corrupted in /Users/gavin/code/dev/storage/mars/data/agent/log/upload/streaming/default. filesessions/fileasync.go:302
2024-01-24T15:29:51-08:00 DEBU [UPLOAD]    Failed to close stream. error:[
ERROR REPORT:
Original Error: *interceptors.RemoteError EOF
Stack Trace:
	github.com/gravitational/teleport/api/client/auditstreamer.go:87 github.com/gravitational/teleport/api/client.(*auditStreamer).Close
	github.com/gravitational/teleport/lib/events/filesessions/fileasync.go:517 github.com/gravitational/teleport/lib/events/filesessions.(*Uploader).upload.func2
	github.com/gravitational/teleport/lib/events/filesessions/fileasync.go:591 github.com/gravitational/teleport/lib/events/filesessions.(*Uploader).upload
	github.com/gravitational/teleport/lib/events/filesessions/fileasync.go:461 github.com/gravitational/teleport/lib/events/filesessions.(*Uploader).startUpload.func1
	runtime/asm_arm64.s:1197 runtime.goexit
User Message: EOF] filesessions/fileasync.go:519
2024-01-24T15:29:51-08:00 DEBU [UPLOAD]    Session upload completed. duration:13.650583ms session-id:432037ec-3dd4-46b1-964f-a2c28fa9352e filesessions/fileasync.go:470

Note the inconsistency between "failed to close stream" and "session upload completed".

In fact, what happened is that the upload completed and the upstream auth server closed the stream, but when we check the error returned from stream.Close(ctx), it's actually wrapped in a RemoteError wrapper created by the api client's grpc stream interceptor.

Fixed by using errors.Is instead, since RemoteError implements Unwrap() error.

@github-actions github-actions Bot added audit-log Issues related to Teleports Audit Log size/sm labels Jan 25, 2024
@github-actions github-actions Bot requested review from lxea and zmb3 January 25, 2024 01:18
@github-actions
Copy link
Copy Markdown
Contributor

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@github-actions
Copy link
Copy Markdown
Contributor

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@GavinFrazar GavinFrazar force-pushed the gavinfrazar/fix-session-streamer-error-logging branch from 8b24f74 to 5a86449 Compare January 25, 2024 01:22
@GavinFrazar GavinFrazar added the no-changelog Indicates that a PR does not require a changelog entry label Jan 25, 2024
@GavinFrazar
Copy link
Copy Markdown
Contributor Author

I guess RemoteError was only added in #36866

However, I will backport to v13/v14 anyway

@GavinFrazar GavinFrazar requested a review from greedy52 January 30, 2024 01:33
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from lxea January 30, 2024 14:42
@GavinFrazar GavinFrazar added this pull request to the merge queue Jan 30, 2024
Merged via the queue into master with commit 1ab467b Jan 30, 2024
@GavinFrazar GavinFrazar deleted the gavinfrazar/fix-session-streamer-error-logging branch January 30, 2024 19:04
@public-teleport-github-review-bot
Copy link
Copy Markdown

@GavinFrazar See the table below for backport results.

Branch Result
branch/v13 Create PR
branch/v14 Create PR
branch/v15 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

audit-log Issues related to Teleports Audit Log no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants