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

Remove trail package #112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove trail package #112

wants to merge 1 commit into from

Conversation

rosstimothy
Copy link
Contributor

@rosstimothy rosstimothy commented Jan 14, 2025

The trail package is the only reason that grpc-go was a dependency of trace. grpc-go is quite a heavy dependency and can cause problems for downstream consumers which don't make use of trail, see https://github.com/vulcand/predicate/blob/master/go.mod#L5-L8.

By moving trail from gravitational/trace to gravitational/teleport/api the functionality will still be available in a consumable module while drastically reducing the functionality and size of trace. Since api already depends on grpc-go the move should not impact the dependency tree there any.

From an audit of all non-teleport forks/mirrors on GitHub I could not find any direct dependents of trace that actually made use of trail. So while removing the package is a breaking change the impact should be minimal for the community at large.

The trail package is the only reason that grpc-go was a
dependency of trace. grpc-go is quite a heavy dependency and can
cause problems for downstream consumers which don't make use of
trail, see https://github.com/vulcand/predicate/blob/master/go.mod#L5-L8.

By moving trail from gravitational/trace to gravitational/teleport/api
the functionality will still be available in a consumable module
while drastically reducing the functionality and size of trace.
Since api already depends on grpc-go the move should not impact the
dependency tree there any.

From an audit of all non-teleport forks/mirrors on GitHub I could
not find any direct dependents of trace that actually made use of
trail. So while removing the package is a breaking change the
impact should be minimal for the community at large.
rosstimothy added a commit to gravitational/teleport that referenced this pull request Jan 14, 2025
Pulling in the trail package directly in api will allow the trace
module to shed the grpc-go dependency. This needs to land prior
to gravitational/trace#112 being included
in a new version of trace.

There should be no noticable change in the api depdency tree since
it already depends on grpc-go. Some additional items from the
trace/internal package were also vendored within trail as needed.
Additionally, some of the public api of trail that was not being
consumed has been made private.
@rosstimothy
Copy link
Contributor Author

This accompanied by gravitational/teleport#51032 and https://github.com/gravitational/teleport.e/pull/5843 should allow trail to be removed without breaking gravitational/teleport.

@rosstimothy rosstimothy marked this pull request as ready for review January 14, 2025 18:16
github-merge-queue bot pushed a commit to gravitational/teleport that referenced this pull request Jan 14, 2025
* Vendor gravitational/trace/trail in api

Pulling in the trail package directly in api will allow the trace
module to shed the grpc-go dependency. This needs to land prior
to gravitational/trace#112 being included
in a new version of trace.

There should be no noticable change in the api depdency tree since
it already depends on grpc-go. Some additional items from the
trace/internal package were also vendored within trail as needed.
Additionally, some of the public api of trail that was not being
consumed has been made private.

* fix: appease linters
github-merge-queue bot pushed a commit to gravitational/teleport that referenced this pull request Jan 14, 2025
* Vendor gravitational/trace/trail in api

Pulling in the trail package directly in api will allow the trace
module to shed the grpc-go dependency. This needs to land prior
to gravitational/trace#112 being included
in a new version of trace.

There should be no noticable change in the api depdency tree since
it already depends on grpc-go. Some additional items from the
trace/internal package were also vendored within trail as needed.
Additionally, some of the public api of trail that was not being
consumed has been made private.

* fix: appease linters
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.

4 participants