envoy: Envoy protos should not have a dependency on opencensus proto

WARNING: If you want to report crashes, leaking of sensitive information, and/or other security issues, please consider reporting them using appropriate channels.

Bug Template

Title: Envoy protos should not have a dependency on opencensus proto

Description: We use Envoy protos for using the xDS APIs between our clients and management server. Until recently things were okay but now there is a dependency on opencensus/proto/trace/v1/trace_config.proto which is unfortunate. The dependency exists because envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto includes third_party/envoy/src/api/envoy/config/trace/v2/trace.proto which now includes third_party/opencensus_proto/trace/v1/trace_config.proto . I think http_connection_manager.proto is a common file included from multiple “core” files.

This has started happening recently probably with the PR https://github.com/envoyproxy/envoy/pull/10324 Repro steps: Without installing the opencensus protos when I try to compile base.proto I get the following compile errors:

Execution failed for task ':grpc-xds:generateProto'.
  opencensus/proto/trace/v1/trace_config.proto: File not found.
  envoy/config/trace/v2/trace.proto:11:1: Import "opencensus/proto/trace/v1/trace_config.proto" was not found or had errors.
  envoy/config/trace/v2/trace.proto:168:3: "opencensus.proto.trace.v1.TraceConfig" is not defined.
  envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto:10:1: Import "envoy/config/trace/v2/trace.proto" was not found or had errors.

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 18 (18 by maintainers)

Commits related to this issue

api: move configuration of tracer provider implementations out of `config.trace` into `extensions.tracers.<provider>` (#10743) Split Tracing.Http and provider-specific configurations out of trace.pro... — committed to envoyproxy/envoy by yskopets 4 years ago

Most upvoted comments

@yskopets thanks a lot! When we have some time we’ll go ahead and reimport these protos and verify we don’t have those dependencies anymore.

sanjaypujare on Apr 16, 2020

@yskopets FWIW, it’s totally fine to move the protos to distinct files inside existing packages within a major version; this doesn’t constitute a breaking API change. Moving across packages does though, so we need to do that only in v4alpha via the move_to_package file-level option.

htuch on Apr 13, 2020

Yes, agreed, we should move trace providers to reflect the regular extension structure.

htuch on Apr 10, 2020