Skip to content

Conversation

@kolaworld
Copy link

@kolaworld kolaworld commented Apr 7, 2025

  • Add https://github.com/ccoveille/go-safecast to fix int casting errors

    • Impact may be slightly breaking for casting in telemetry span attributes.

    • Specifically, if casting int fails, we fallback to setting the span attribute as a string using formatting rather than dropping the span attribute.

    • Pro of fallback: is we don't drop the span attribute no matter what.

    • Con of fallback: is querying span attributes may break given type change

    • See e18436c

  • Add //nolint:gosec to resolve lint errors for casting int

  • Add //nolint:sqlclosecheck to resolve lint errors given the lint failures are around wrapping database/sql methods. i.e. we're not supposed to close those.

Copy link
Member

@InfernoJJ InfernoJJ left a comment

Choose a reason for hiding this comment

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

I don't like this as its making the observability code the main focus of the function due to the number of lines it'll take up and the repetetive nature. It also seems like we're upcasting an int to int64 which will fit and thus not ever trip the string version.

Seems like we enabled another buggy linter.

@kolaworld kolaworld requested a review from InfernoJJ April 7, 2025 17:48
@kolaworld
Copy link
Author

cc @InfernoJJ updated to use simpler nolint directive

@kolaworld kolaworld merged commit 063c6a1 into master Apr 7, 2025
15 checks passed
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.

3 participants