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!(rm-cause): causes are harmful #620

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AndrewWinterman
Copy link
Contributor

This code is demented. It produces messages like the folliowing
(irrelvant fields redacted):

{
	"content": {
		"attributes": {
			"error": {
				"kind": "error",
				"cause": {
					"kind": "cause",
					"cause": {
						"kind": "cause",
						"cause": {
							"kind": "cause",
							"message": "10.96.232.125:5432 (app2a-callstatemanager-xpmuvqngpcd.cluster-ro-cntwmzilna0e.us-east-1.rds.amazonaws.com): failed to write startup message: context canceled"
						},
						"message": "failed to connect to `user=readwrite database=callstatemanager`"
					},
					"message": "failed to connect to `user=readwrite database=callstatemanager`: 10.96.232.125:5432 (app2a-callstatemanager-xpmuvqngpcd.cluster-ro-cntwmzilna0e.us-east-1.rds.amazonaws.com): failed to write startup message: context canceled getting connection to host app2a-callstatemanager-xpmuvqngpcd.cluster-ro-cntwmzilna0e.us-east-1.rds.amazonaws.com:5432 for org a5fc3e3c-b376-4327-9d80-3e29fe62e6e5"
				},
				"error": "failed to connect to `user=readwrite database=callstatemanager`: 10.96.232.125:5432 (app2a-callstatemanager-xpmuvqngpcd.cluster-ro-cntwmzilna0e.us-east-1.rds.amazonaws.com): failed to write startup message: context canceled getting connection to host app2a-callstatemanager-xpmuvqngpcd.cluster-ro-cntwmzilna0e.us-east-1.rds.amazonaws.com:5432 for org a5fc3e3c-b376-4327-9d80-3e29fe62e6e5",
				"message": ""
			},
		}
	}
}

In order to query this, you end up writing statements like
error.cause.cause.cause.cause.message

There's no point in that. All of our tools also support "in" queries,
so you can just check if "context cancelled" in error.error
(error.error) is also semantically problematic, buit whatvs.

All this does is inflate our log volume and increase meaninglessly the
number of columns in honeycomb queries.

Remove it and profit.

NB: this is a scorched earch approach. Anyone updating would be
immediately su bject to this new behavior, and it might break alerting
if someone was specifically looking for the 5th cause or something goofy
like that.

Please read CONTRIBUTING.md for additional information on contributing to this repository!

What this PR does / why we need it

Jira ID

[XX-XX]

Notes for your reviewers

This code is demented. It produces messages like the folliowing
(irrelvant fields redacted):

```json
{
	"content": {
		"attributes": {
			"error": {
				"kind": "error",
				"cause": {
					"kind": "cause",
					"cause": {
						"kind": "cause",
						"cause": {
							"kind": "cause",
							"message": "10.96.232.125:5432 (app2a-callstatemanager-xpmuvqngpcd.cluster-ro-cntwmzilna0e.us-east-1.rds.amazonaws.com): failed to write startup message: context canceled"
						},
						"message": "failed to connect to `user=readwrite database=callstatemanager`"
					},
					"message": "failed to connect to `user=readwrite database=callstatemanager`: 10.96.232.125:5432 (app2a-callstatemanager-xpmuvqngpcd.cluster-ro-cntwmzilna0e.us-east-1.rds.amazonaws.com): failed to write startup message: context canceled getting connection to host app2a-callstatemanager-xpmuvqngpcd.cluster-ro-cntwmzilna0e.us-east-1.rds.amazonaws.com:5432 for org a5fc3e3c-b376-4327-9d80-3e29fe62e6e5"
				},
				"error": "failed to connect to `user=readwrite database=callstatemanager`: 10.96.232.125:5432 (app2a-callstatemanager-xpmuvqngpcd.cluster-ro-cntwmzilna0e.us-east-1.rds.amazonaws.com): failed to write startup message: context canceled getting connection to host app2a-callstatemanager-xpmuvqngpcd.cluster-ro-cntwmzilna0e.us-east-1.rds.amazonaws.com:5432 for org a5fc3e3c-b376-4327-9d80-3e29fe62e6e5",
				"message": ""
			},
		}
	}
}
```

In order to query this, you end up writing statements like
`error.cause.cause.cause.cause.message`

There's no point in that. All of our tools also support "in" queries,
so you can just check `if "context cancelled" in error.error`
(error.error) is also semantically problematic, buit whatvs.

All this does is inflate our log volume and increase meaninglessly the
number of columns in honeycomb queries.

Remove it and profit.

NB: this is a scorched earch approach. Anyone updating would be
immediately su bject to this new behavior, and it might break alerting
if someone was specifically looking for the 5th cause or something goofy
like that.
@@ -168,15 +161,6 @@ func nestInfo(err error) *nestedErrorInfo {
Stack: errStack(err),
Custom: custom,
}
// obtain nested error, collapsing upward if possible.
Copy link
Contributor

@asms asms Feb 1, 2025

Choose a reason for hiding this comment

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

Is this nestInfo function still needed? Maybe we can remove the whole function.

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