Remove glog dependency

[ash2k]

This is a follow up for #3361 which hasn't removed all the glog references.

References to other Issues or PRs

#3342

Have you read the Contributing Guidelines?

Yes.

Brief description of what is fixed or changed

Replaces glog with grpclog.

Other comments

Summary by CodeRabbit

• Refactor: Replaced the deprecated logging package glog with grpclog across the entire codebase. This change enhances the reliability of the logging system and aligns with the latest standards.
• Chore: Removed the logtostderr=true option from various protoc commands, simplifying the command-line interface and reducing potential confusion for users.
• Documentation: Updated the documentation to reflect the changes in logging and command-line options, making it easier for users to understand and use the system.

[coderabbitai]

Walkthrough

The changes across the codebase primarily involve the replacement of the deprecated glog logging package with the grpclog package from gRPC. This includes updates to import statements, logging function calls, and build dependencies. Additionally, the logtostderr option has been removed from various protoc commands.

Changes

Files	Summary
README.md, docs/.../installation_for_cygwin.md	Updated protoc commands, removed glog import, and replaced with grpclog.
examples/internal/cmd/.../BUILD.bazel, examples/internal/gateway/BUILD.bazel, examples/internal/integration/BUILD.bazel, examples/internal/server/BUILD.bazel, internal/descriptor/BUILD.bazel, internal/httprule/BUILD.bazel	Replaced glog dependency with grpclog in build files.
examples/internal/cmd/.../main.go, examples/internal/gateway/main.go, examples/internal/integration/main_test.go, examples/internal/server/main.go, internal/descriptor/registry.go	Replaced glog import with grpclog, updated logging function calls.
examples/internal/gateway/handlers.go, examples/internal/server/a_bit_of_everything.go, examples/internal/server/echo.go, examples/internal/server/non_standard_names.go, examples/internal/server/unannotatedecho.go, internal/descriptor/services.go, internal/httprule/parse_test.go	Replaced glog import with grpclog, updated logging function calls.
protoc-gen-grpc-gateway/main.go, protoc-gen-openapiv2/main.go	Added new "logtostderr" flag marked as a no-op.
docs/.../customizing_openapi_output.md, docs/.../grpc_api_configuration.md, examples/internal/browser/gulpfile.js, protoc-gen-grpc-gateway/BUILD.bazel, protoc-gen-openapiv2/defs.bzl	Removed logtostderr option from various commands and build rules.

"In the land of code, where the shadows log,
A rabbit hopped, replacing glog.
With grpclog now, the messages flow,
Clear as the stream, where the willows grow.
No more logtostderr, it had its day,
In the pull request, the changes lay.
So hop along now, there's nothing to fear,
For the CodeRabbit 🐇 has made things clear!"

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• Mention @coderabbitai in any review comment for bot assistance.
• Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

• Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
• Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
  resume reviews, enter @coderabbitai resume as any issue or review comment.

[coderabbitai]

Review Status

Actionable comments generated: 0

Commits

Files that changed from the base of the PR and between af5e76e and be46f0e.

Files ignored due to filter (2)

• go.mod
• go.sum

Files selected for processing (22)

• README.md (2 hunks)
• docs/docs/development/installation_for_cygwin.md (1 hunks)
• examples/internal/cmd/example-gateway-server/BUILD.bazel (1 hunks)
• examples/internal/cmd/example-gateway-server/main.go (3 hunks)
• examples/internal/cmd/example-grpc-server/BUILD.bazel (1 hunks)
• examples/internal/cmd/example-grpc-server/main.go (2 hunks)
• examples/internal/gateway/BUILD.bazel (1 hunks)
• examples/internal/gateway/handlers.go (2 hunks)
• examples/internal/gateway/main.go (3 hunks)
• examples/internal/integration/BUILD.bazel (1 hunks)
• examples/internal/integration/main_test.go (4 hunks)
• examples/internal/server/BUILD.bazel (1 hunks)
• examples/internal/server/a_bit_of_everything.go (15 hunks)
• examples/internal/server/echo.go (3 hunks)
• examples/internal/server/main.go (3 hunks)
• examples/internal/server/non_standard_names.go (2 hunks)
• examples/internal/server/unannotatedecho.go (3 hunks)
• internal/descriptor/BUILD.bazel (1 hunks)
• internal/descriptor/registry.go (4 hunks)
• internal/descriptor/services.go (7 hunks)
• internal/httprule/BUILD.bazel (1 hunks)
• internal/httprule/parse_test.go (2 hunks)

Files skipped from review due to trivial changes (18)

• docs/docs/development/installation_for_cygwin.md
• examples/internal/cmd/example-gateway-server/BUILD.bazel
• examples/internal/cmd/example-gateway-server/main.go
• examples/internal/cmd/example-grpc-server/BUILD.bazel
• examples/internal/cmd/example-grpc-server/main.go
• examples/internal/gateway/BUILD.bazel
• examples/internal/gateway/handlers.go
• examples/internal/gateway/main.go
• examples/internal/integration/BUILD.bazel
• examples/internal/integration/main_test.go
• examples/internal/server/a_bit_of_everything.go
• examples/internal/server/echo.go
• examples/internal/server/main.go
• examples/internal/server/unannotatedecho.go
• internal/descriptor/BUILD.bazel
• internal/descriptor/services.go
• internal/httprule/BUILD.bazel
• internal/httprule/parse_test.go

Additional comments (Suppressed): 10

examples/internal/server/non_standard_names.go (2)

• 3-8: The import of glog has been replaced with grpclog. Ensure that grpclog provides the same or better functionality as glog and that it meets the project's requirements.

• 38-38: The logging calls have been updated from glog.Info to grpclog.Info. Ensure that the logging levels and formats are consistent with the rest of the project.

README.md (2)

• 357-366: The import statement for glog has been replaced with grpclog. Ensure that the grpclog package is added to the project dependencies.

• 391-398: The glog.Flush() call has been removed as grpclog does not require explicit flushing. Also, glog.Fatal(err) has been replaced with grpclog.Fatal(err). Ensure that all instances of glog have been replaced with grpclog throughout the codebase.

examples/internal/server/BUILD.bazel (1)

• 23-35: The glog dependency has been replaced with grpclog as part of the logging library switch. Ensure that grpclog is compatible with the rest of the codebase and that it has been properly imported in all the necessary files.

internal/descriptor/registry.go (5)

• 8-14: The import statement for glog has been replaced with grpclog. Ensure that grpclog is compatible with the rest of the codebase and provides the necessary logging functionality.

• 286-288: The logging function glog.V(1).Infof has been replaced with grpclog.V(1) and grpclog.Infof. Ensure that the verbosity level and the logging format are preserved with the new logging function.

-		glog.V(1).Infof("register name: %s", m.FQMN())
+		if grpclog.V(1) {
+			grpclog.Infof("Register name: %s", m.FQMN())
+		}

• 309-311: The logging function glog.V(1).Infof has been replaced with grpclog.V(1) and grpclog.Infof. Ensure that the verbosity level and the logging format are preserved with the new logging function.

-		glog.V(1).Infof("register enum name: %s", e.FQEN())
+		if grpclog.V(1) {
+			grpclog.Infof("Register enum name: %s", e.FQEN())
+		}

• 318-320: The logging function glog.V(1).Infof has been replaced with grpclog.V(1) and grpclog.Infof. Ensure that the verbosity level and the logging format are preserved with the new logging function.

-		glog.V(1).Infof("lookup %s from %s", name, location)
+		if grpclog.V(1) {
+			grpclog.Infof("Lookup %s from %s", name, location)
+		}

• 346-348: The logging function glog.V(1).Infof has been replaced with grpclog.V(1) and grpclog.Infof. Ensure that the verbosity level and the logging format are preserved with the new logging function.

-		glog.V(1).Infof("lookup enum %s from %s", name, location)
+		if grpclog.V(1) {
+			grpclog.Infof("Lookup enum %s from %s", name, location)
+		}

[johanbrandhorst]

Nice!

[coderabbitai]

Review Status

Actionable comments generated: 0

Commits

Files that changed from the base of the PR and between bfa6d9e and 9332e6f.

Files ignored due to filter (2)

• go.mod
• go.sum

Files selected for processing (22)

• README.md (2 hunks)
• docs/docs/development/installation_for_cygwin.md (1 hunks)
• examples/internal/cmd/example-gateway-server/BUILD.bazel (1 hunks)
• examples/internal/cmd/example-gateway-server/main.go (3 hunks)
• examples/internal/cmd/example-grpc-server/BUILD.bazel (1 hunks)
• examples/internal/cmd/example-grpc-server/main.go (2 hunks)
• examples/internal/gateway/BUILD.bazel (1 hunks)
• examples/internal/gateway/handlers.go (2 hunks)
• examples/internal/gateway/main.go (3 hunks)
• examples/internal/integration/BUILD.bazel (1 hunks)
• examples/internal/integration/main_test.go (4 hunks)
• examples/internal/server/BUILD.bazel (1 hunks)
• examples/internal/server/a_bit_of_everything.go (15 hunks)
• examples/internal/server/echo.go (3 hunks)
• examples/internal/server/main.go (3 hunks)
• examples/internal/server/non_standard_names.go (2 hunks)
• examples/internal/server/unannotatedecho.go (3 hunks)
• internal/descriptor/BUILD.bazel (1 hunks)
• internal/descriptor/registry.go (4 hunks)
• internal/descriptor/services.go (7 hunks)
• internal/httprule/BUILD.bazel (1 hunks)
• internal/httprule/parse_test.go (4 hunks)

Files skipped from review due to trivial changes (17)

• docs/docs/development/installation_for_cygwin.md
• examples/internal/cmd/example-gateway-server/BUILD.bazel
• examples/internal/cmd/example-gateway-server/main.go
• examples/internal/cmd/example-grpc-server/BUILD.bazel
• examples/internal/cmd/example-grpc-server/main.go
• examples/internal/gateway/BUILD.bazel
• examples/internal/gateway/handlers.go
• examples/internal/gateway/main.go
• examples/internal/integration/main_test.go
• examples/internal/server/a_bit_of_everything.go
• examples/internal/server/echo.go
• examples/internal/server/main.go
• examples/internal/server/non_standard_names.go
• internal/descriptor/registry.go
• internal/descriptor/services.go
• internal/httprule/BUILD.bazel
• internal/httprule/parse_test.go

Additional comments (Suppressed): 7

examples/internal/integration/BUILD.bazel (1)

• 24-24: The dependency @com_github_golang_glog//:glog has been replaced with @org_golang_google_grpc//grpclog. Ensure that this change does not affect the functionality of the code and that grpclog provides all the necessary features that glog did.

internal/descriptor/BUILD.bazel (1)

• 24-24: The dependency @com_github_golang_glog//:glog has been replaced with @org_golang_google_grpc//grpclog. Ensure that this change does not affect the functionality of the code and that all necessary updates have been made in the codebase to accommodate this change.

examples/internal/server/BUILD.bazel (1)

• 23-35: The dependency @com_github_golang_glog//:glog has been replaced with @org_golang_google_grpc//grpclog. Ensure that this change does not affect the build process and that all logging calls in the codebase have been updated to use grpclog instead of glog.

examples/internal/server/unannotatedecho.go (2)

• 3-10: The import of github.com/golang/glog has been replaced with google.golang.org/grpc/grpclog. Ensure that grpclog provides the same or better functionality as glog and that it is compatible with the rest of the codebase.

• 39-40: The logging function glog.Info(msg) has been replaced with grpclog.Info(msg). Ensure that the new logging function provides the same or better functionality as the old one and that it is compatible with the rest of the codebase.

README.md (2)

• 357-366: The import of github.com/golang/glog has been replaced with google.golang.org/grpc/grpclog. This aligns with the PR's goal of replacing glog with grpclog. Ensure that the grpclog package is included in the project dependencies.

• 395-397: The logging function glog.Fatal(err) has been replaced with grpclog.Fatal(err). This is consistent with the replacement of glog with grpclog. However, note that the defer glog.Flush() statement has been removed. This is acceptable because grpclog does not require explicit flushing.

[coderabbitai]

Review Status

Actionable comments generated: 0

Commits

Files that changed from the base of the PR and between bfa6d9e and cc2bd20.

Files ignored due to filter (2)

• go.mod
• go.sum

Files selected for processing (22)

• README.md (2 hunks)
• docs/docs/development/installation_for_cygwin.md (1 hunks)
• examples/internal/cmd/example-gateway-server/BUILD.bazel (1 hunks)
• examples/internal/cmd/example-gateway-server/main.go (3 hunks)
• examples/internal/cmd/example-grpc-server/BUILD.bazel (1 hunks)
• examples/internal/cmd/example-grpc-server/main.go (2 hunks)
• examples/internal/gateway/BUILD.bazel (1 hunks)
• examples/internal/gateway/handlers.go (2 hunks)
• examples/internal/gateway/main.go (3 hunks)
• examples/internal/integration/BUILD.bazel (1 hunks)
• examples/internal/integration/main_test.go (4 hunks)
• examples/internal/server/BUILD.bazel (1 hunks)
• examples/internal/server/a_bit_of_everything.go (15 hunks)
• examples/internal/server/echo.go (3 hunks)
• examples/internal/server/main.go (3 hunks)
• examples/internal/server/non_standard_names.go (2 hunks)
• examples/internal/server/unannotatedecho.go (3 hunks)
• internal/descriptor/BUILD.bazel (1 hunks)
• internal/descriptor/registry.go (4 hunks)
• internal/descriptor/services.go (7 hunks)
• internal/httprule/BUILD.bazel (1 hunks)
• internal/httprule/parse_test.go (4 hunks)

Files skipped from review due to trivial changes (17)

• docs/docs/development/installation_for_cygwin.md
• examples/internal/cmd/example-gateway-server/BUILD.bazel
• examples/internal/cmd/example-gateway-server/main.go
• examples/internal/cmd/example-grpc-server/main.go
• examples/internal/gateway/BUILD.bazel
• examples/internal/gateway/handlers.go
• examples/internal/gateway/main.go
• examples/internal/integration/main_test.go
• examples/internal/server/a_bit_of_everything.go
• examples/internal/server/echo.go
• examples/internal/server/main.go
• examples/internal/server/non_standard_names.go
• examples/internal/server/unannotatedecho.go
• internal/descriptor/registry.go
• internal/descriptor/services.go
• internal/httprule/BUILD.bazel
• internal/httprule/parse_test.go

Additional comments (Suppressed): 7

examples/internal/cmd/example-grpc-server/BUILD.bazel (1)

• 8-12: The dependency @com_github_golang_glog//:glog has been replaced with @org_golang_google_grpc//grpclog. Ensure that the new dependency is correctly set up in the Bazel workspace file and that it doesn't break the build.

examples/internal/integration/BUILD.bazel (1)

• 24-24: The dependency @com_github_golang_glog//:glog has been replaced with @org_golang_google_grpc//grpclog. Ensure that this change does not affect the functionality of the code and that grpclog provides all the necessary features that were previously provided by glog.

examples/internal/server/BUILD.bazel (1)

• 23-35: The dependency @com_github_golang_glog//:glog has been replaced with @org_golang_google_grpc//grpclog. Ensure that this change does not affect the functionality of the code and that grpclog provides all the necessary features that glog did.

internal/descriptor/BUILD.bazel (1)

• 24-24: The dependency @com_github_golang_glog//:glog has been replaced with @org_golang_google_grpc//grpclog. Ensure that this change doesn't introduce any compatibility issues and that grpclog provides all the necessary functionality previously provided by glog.

README.md (3)

• 357-366: The import of github.com/golang/glog has been replaced with google.golang.org/grpc/grpclog. Ensure that all logging function calls have been updated to use grpclog instead of glog.

• 395-397: The glog.Fatal(err) has been replaced with grpclog.Fatal(err). This change is consistent with the replacement of glog with grpclog.

• 392-398: The defer glog.Flush() has been removed. This is acceptable as grpclog does not require explicit flushing.

[johanbrandhorst]

Not sure about that Bazel build error, the flag should still exist? Or does removing glog remove the flag? Maybe we need to add the flag back?

[coderabbitai]

Review Status

Actionable comments generated: 0

Commits

Files that changed from the base of the PR and between cc2bd20 and e8f5c7d.

Files selected for processing (7)

• README.md (7 hunks)
• docs/docs/development/installation_for_cygwin.md (2 hunks)
• docs/docs/mapping/customizing_openapi_output.md (1 hunks)
• docs/docs/mapping/grpc_api_configuration.md (2 hunks)
• examples/internal/browser/gulpfile.js (1 hunks)
• protoc-gen-grpc-gateway/BUILD.bazel (1 hunks)
• protoc-gen-openapiv2/defs.bzl (1 hunks)

Files skipped from review due to trivial changes (7)

• README.md
• docs/docs/development/installation_for_cygwin.md
• docs/docs/mapping/customizing_openapi_output.md
• docs/docs/mapping/grpc_api_configuration.md
• examples/internal/browser/gulpfile.js
• protoc-gen-grpc-gateway/BUILD.bazel
• protoc-gen-openapiv2/defs.bzl

[ash2k]

@johanbrandhorst The flag was part of glog, it was added by the library. Removing glog removed the flag too. I personally don't think the flag is needed, but this might be a breaking change for users when they upgrade the version. It's trivial to fix though - remove the flag from where it's set.

[johanbrandhorst]

I see, yes, it will be a breaking change, and I think we're best of just including a new flag with the same purpose in our application so as not to break users. Would you mind adding a new flag for this? Thanks.

[ash2k]

I don't think we can include such a flag as it will conflict with glog's flag if user's application is using glog.

[johanbrandhorst]

This is protoc-gen-grpc-gateway, we shouldn't have anyone using it as a library, so it should be fine to add, I should think?

[ash2k]

@johanbrandhorst What about other flags that users might have been setting? See https://pkg.go.dev/github.com/golang/glog#pkg-overview

Also, strictly speaking turning flags into no-ops is also a breaking change. Silently not doing what they used to do is even worse than failing explicitly in my opinion.

Honestly, I don't think many (if anyone) would notice the removal. Again, fixing the problem is trivial for the user but this "fix" to add the flags is clearly a hack. I'd personally not do it.

[johanbrandhorst]

Believe me, I'd rather not do it too. I do think it would have a pretty big impact on our users though. During much of the gateways existence the canonical example of the use of the generator has included the use of the logtostderr flag. I believe many of our users out there set this flag without really caring about what it does just because the README. As a result, I think we need to add this particular flag back, and honestly it could just be a noop. If other flags are used by users we can add them back as we get bug reports about it, but I think this will affect a large number of users. This codesearch shows that it's quite widely used (and in popular repositories): https://github.com/search?type=code&auto_enroll=true&q=%22logtostderr%3Dtrue%2C%22. I think we'll actually need to add it back to both protoc-gen-grpc-gateway and protoc-gen-openapiv2. Again, I'd prefer this wasn't the case, but I want to be conscious of our users. What do you think?

[ash2k]

I'll add the flag. I don't use either of the projects you mentioned directly, but I have them as transitive deps (for some reason).

If I used the project, I'd rather cleaned up the invocation than accumulated useless cruft but 🤷

[coderabbitai]

Review Status

Actionable comments generated: 0

Commits

Files that changed from the base of the PR and between e8f5c7d and 0c89b1e.

Files selected for processing (2)

• protoc-gen-grpc-gateway/main.go (1 hunks)
• protoc-gen-openapiv2/main.go (1 hunks)

Files skipped from review due to trivial changes (2)

• protoc-gen-grpc-gateway/main.go
• protoc-gen-openapiv2/main.go

[johanbrandhorst]

Thanks a lot for adding back the flag, I appreciate it 👍🏻. Some minor questions.

[coderabbitai]

Review Status

Actionable comments generated: 0

Commits

Files that changed from the base of the PR and between cc2bd20 and 30a06a5.

Files selected for processing (9)

• README.md (7 hunks)
• docs/docs/development/installation_for_cygwin.md (2 hunks)
• docs/docs/mapping/customizing_openapi_output.md (1 hunks)
• docs/docs/mapping/grpc_api_configuration.md (2 hunks)
• examples/internal/browser/gulpfile.js (1 hunks)
• protoc-gen-grpc-gateway/BUILD.bazel (1 hunks)
• protoc-gen-grpc-gateway/main.go (1 hunks)
• protoc-gen-openapiv2/defs.bzl (1 hunks)
• protoc-gen-openapiv2/main.go (1 hunks)

Files skipped from review due to trivial changes (9)

• README.md
• docs/docs/development/installation_for_cygwin.md
• docs/docs/mapping/customizing_openapi_output.md
• docs/docs/mapping/grpc_api_configuration.md
• examples/internal/browser/gulpfile.js
• protoc-gen-grpc-gateway/BUILD.bazel
• protoc-gen-grpc-gateway/main.go
• protoc-gen-openapiv2/defs.bzl
• protoc-gen-openapiv2/main.go

[johanbrandhorst]

LGTM

[johanbrandhorst]

Thanks for your contribution!

[ash2k]

Happy this got merged quickly! Thank you!