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

Better handling of OTLP proto sources #652

Closed
pjanotti opened this issue Apr 30, 2020 · 4 comments
Closed

Better handling of OTLP proto sources #652

pjanotti opened this issue Apr 30, 2020 · 4 comments
Labels
enhancement New feature or request help wanted Good for taking. Extra help will be provided by maintainers infra Infra work - CI/CD, code coverage, linters pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package

Comments

@pjanotti
Copy link
Contributor

Separating this from the discussion at #19

Currently, the proto files are copied to the OTLP exporter sources. At this stage I see two possible options:

  1. Use a github submodule;
  2. Hack the project to use a nuget source package - of course, in the end, this will require the proto repo to be published in a nuget.
@pjanotti pjanotti added the enhancement New feature or request label Apr 30, 2020
@reyang reyang added pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package infra Infra work - CI/CD, code coverage, linters release:required-for-ga help wanted Good for taking. Extra help will be provided by maintainers labels Jul 18, 2020
@pcwiese
Copy link
Contributor

pcwiese commented Aug 3, 2020

So I've got a change that swaps out the protos with files from a git submodule but I don't know how to incorporate this into the ci workflows. A basic clone of the repo won't pull submodules by default (I think). You would need to specify the --recursive flag to the step in some way. @eddynaka Do you know offhand what additional changes would have to be done?

Here is my branch with the change:
https://github.com/pcwiese/opentelemetry-dotnet/tree/pwiese/otlp-proto-via-git-submodule

After checking this in, developers with existing repos would also have to do:

git submodule init
git submodule update

Otherwise, the submodule files won't be there.

@eddynaka
Copy link
Contributor

eddynaka commented Aug 3, 2020

Talked with @pcwiese offline, but, GitHub actions does support that: https://github.com/actions/checkout

We have to add:

- uses: actions/checkout@v2
  with:
    submodules: 'recursive'

@pcwiese
Copy link
Contributor

pcwiese commented Aug 6, 2020

We are not going to use Git submodules for this. if we need to, we can create a NuGet package

@martinjt
Copy link
Member

I think this is pretty solid now, and I'm not sure there are intentions to do anything else.

I'd suggest reopening this if you feel there is a need to do this with some more up to date reasoning.

@martinjt martinjt closed this as not planned Won't fix, can't repro, duplicate, stale Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Good for taking. Extra help will be provided by maintainers infra Infra work - CI/CD, code coverage, linters pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package
Projects
None yet
6 participants