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 loglevel-debug and DEBUG=apollo-gateway:* support. #3896

Merged
merged 6 commits into from
Mar 18, 2020

Conversation

abernix
Copy link
Member

@abernix abernix commented Mar 17, 2020

While this package provided some simple functionality at one point or another, it was neither actively maintained, properly typed, and as of # fully-featured as of #3894, no longer really necessary. Also, since it re-uses the same logger based on repeat requests to getLogger("name") in a global context, it was responsible for recursively wrapping our log function in tests with the same methodFactory and causing our log messages to get longer and longer for each test which used ApolloGateway. This only affected tests, but was hard to track down the cause!

It also was responsible for prepending the log lines with dates and times which were often undesirable since a more fully functional logger (as in, most cloud-based loggers) would already have baked in such metadata, rather than needing it inlined in the message itself.

The new logger functionality in the above PR should provide much more flexibility here and debug: true on the ApolloGateway constructor can replicate the same behavior.

Anyone who misses this behavior can utilize the new pluggable logger to provide loglevel-debug themselves.

While this package provided some simple functionality at one point or
another, it was neither actively maintained, properly typed, and as of #
fully-featured as of
#3894, no longer really
necessary.  Also, since it re-uses the same logger based on repeat requests
to `getLogger("name")` in a global context, it was responsible for
recursively wrapping our log function in tests with the same `methodFactory`
and causing our log messages to get longer and longer for each test which
used `ApolloGateway`.  This only affected tests, but was hard to track down
the cause!

The new `logger` functionality in the above PR should provide much more
flexibility here and `debug: true` on the `ApolloGateway` constructor can
replicate the same behavior.

Anyone who misses this behavior can utilize the new pluggable
logger to provide [`loglevel-debug`] themselves!

[`loglevel-debug`]: https://npm.im/loglevel-debug
@abernix abernix requested a review from trevor-scheer March 17, 2020 21:52
@abernix abernix changed the title Remove loglevel-debug and DEBUG=apollo-gateway:* support. WIP: Remove loglevel-debug and DEBUG=apollo-gateway:* support. Mar 17, 2020
abernix added 2 commits March 18, 2020 00:01
This was only used for display purposes in a way that turned out to be to
resolve a problem that `loglevel-debug` had necessitated by the way it
provides its own `methodFactory` that prepends the date, etc.
@abernix abernix changed the title WIP: Remove loglevel-debug and DEBUG=apollo-gateway:* support. Remove loglevel-debug and DEBUG=apollo-gateway:* support. Mar 17, 2020
@abernix abernix force-pushed the abernix/remove-gateway-loglevel-debug branch from 90cb1df to a9f1a1f Compare March 17, 2020 22:04
@abernix abernix added this to the Release 2.12.0 milestone Mar 17, 2020
Copy link
Member

@trevor-scheer trevor-scheer left a comment

Choose a reason for hiding this comment

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

YESSSSS 🥇

@abernix abernix merged commit 0ca43f8 into release-2.12.0 Mar 18, 2020
@abernix abernix deleted the abernix/remove-gateway-loglevel-debug branch March 18, 2020 17:32
abernix added a commit that referenced this pull request Jun 29, 2020
Removed in the same spirit as was done in the `@apollo/gateway` package (see
footer for PR reference) due to general deficiencies and bugs in that
chosen implementation and also just because it's widely unutilized (if used
at all).

Much in the same way as the PR below suggests, this behavior could be opted
into if necessary by providing a custom `logger` to the plugin options.
While that option doesn't yet exist, a PR would be very much welcome (though
it does require juggling some of the conditions around how the "dry-run"
mode works and does its logging in a well-defined way.

[Ref]: #3896.
abernix added a commit that referenced this pull request Jun 29, 2020
Removed in the same spirit as was done in the `@apollo/gateway` package (see
footer for PR reference) due to general deficiencies and bugs in that
chosen implementation and also just because it's widely unutilized (if used
at all).

Much in the same way as the PR below suggests, this behavior could be opted
into if necessary by providing a custom `logger` to the plugin options.
While that option doesn't yet exist, a PR would be very much welcome (though
it does require juggling some of the conditions around how the "dry-run"
mode works and does its logging in a well-defined way.

[Ref]: #3896.
abernix added a commit that referenced this pull request Jun 29, 2020
BREAKING CHANGE: Drop the use of uncommonly used `loglevel-debug`.

Removed in the same spirit as was done in the `@apollo/gateway` package (see
footer for PR reference) due to general deficiencies and bugs in that
chosen implementation and also just because it's widely unutilized (if used
at all).

Much in the same way as the PR below suggests, this behavior could be opted
into if necessary by providing a custom `logger` to the plugin options.
While that option doesn't yet exist, a PR would be very much welcome (though
it does require juggling some of the conditions around how the "dry-run"
mode works and does its logging in a well-defined way.

[Ref]: #3896.
abernix added a commit that referenced this pull request Aug 7, 2020
BREAKING CHANGE: Drop the use of uncommonly used `loglevel-debug`.

Removed in the same spirit as was done in the `@apollo/gateway` package (see
footer for PR reference) due to general deficiencies and bugs in that
chosen implementation and also just because it's widely unutilized (if used
at all).

Much in the same way as the PR below suggests, this behavior could be opted
into if necessary by providing a custom `logger` to the plugin options.
While that option doesn't yet exist, a PR would be very much welcome (though
it does require juggling some of the conditions around how the "dry-run"
mode works and does its logging in a well-defined way.

[Ref]: #3896.
abernix added a commit to apollographql/federation that referenced this pull request Sep 4, 2020
…graphql/apollo-server#3896)

While this package provided some simple functionality at one point or
another, it was neither actively maintained, properly typed, and as of #
fully-featured as of
apollographql/apollo-server#3894, no longer really
necessary.  Also, since it re-uses the same logger based on repeat requests
to `getLogger("name")` in a global context, it was responsible for
recursively wrapping our log function in tests with the same `methodFactory`
and causing our log messages to get longer and longer for each test which
used `ApolloGateway`.  This only affected tests, but was hard to track down
the cause!

The new `logger` functionality in the above PR should provide much more
flexibility here and `debug: true` on the `ApolloGateway` constructor can
replicate the same behavior.

Anyone who misses this behavior can utilize the new pluggable
logger to provide [`loglevel-debug`] themselves!
Apollo-Orig-Commit-AS: apollographql/apollo-server@0ca43f8
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants