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

Conflicting inputs on erlang build of Opentelemetry #596

Open
srstrong opened this issue Mar 7, 2024 · 1 comment
Open

Conflicting inputs on erlang build of Opentelemetry #596

srstrong opened this issue Mar 7, 2024 · 1 comment

Comments

@srstrong
Copy link

srstrong commented Mar 7, 2024

Hi,

We use open-telemetry as a third party dependency (https://github.com/open-telemetry/opentelemetry-erlang), but are hitting a problem with the conflicting inputs check in erlang_application. The issue is caused by the opentelemetry_api_experimental and the opentelemetry_experimental both having include files names "otel_metrics.hrl". I get that might not be best practice, but I think the reasoning is that the otel_metrics.hrl inside opentelemetry_experimental is not part of the public API, so in principal outside code shouldn't care what that file is called.

For my rules, I have specified the hols in opentelemetry_experimental as part of "srcs", so they should be private:

erlang_application(
    name = "opentelemetry_experimental",
    srcs = glob(["opentelemetry/apps/opentelemetry_experimental/src/*.erl", "opentelemetry/apps/opentelemetry_experimental/include/*.hrl"]),
    version = "1.2.1",
   ...

and for opentelemetry_experimental_api the hrls are listed in "includes":

erlang_application(
    name = "opentelemetry_api_experimental",
    srcs = glob(["opentelemetry/apps/opentelemetry_api_experimental/src/*.erl"]), 
    includes = glob(["opentelemetry/apps/opentelemetry_api_experimental/include/*.hrl"]),

Would it be reasonable for the conflict checker to take into account the private vs public distinction?

@TheGeorge
Copy link
Contributor

We haven't gotten a great solution for the division between public and private includes yet. The guidance for now is to ensure that header files are named identifiable by their file name regardless of whether they are residing in the public includes/ directory, or in a private source directory.

The issue is as following:

  • we use erlc as build tool, and need to specify all include directories so they can be found following the -include or -include_lib attribute in the code
  • for public headers we enforce them to be included with -include_lib("app/include/public.hrl")
  • if you are in the same app, you can include public headers with -include("public.hrl")
  • public headers can include private ones with -include("private.hrl")

The last point essentially lifts all private headers to be public as well. Ideally, we would like to enforce a better separation, and say that public headers cannot include private ones. But in practice, this would break alot of code. So we error on the side of caution and enforce uniqueness in the name.

However, I think we could add a strict mode, on application level, that enforces a better separation.

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

No branches or pull requests

2 participants