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

Dockerfile for quick demo/troubleshooting purpose. #905

Merged
merged 3 commits into from
Dec 1, 2021

Conversation

lalitb
Copy link
Member

@lalitb lalitb commented Jul 15, 2021

I have been using this Dockerfile for some time as an environment for quick troubleshooting and felt it would act as a good playground for new devs/users to quickly try out opentelemetry-cpp along with exporters ( OTLP, Jaeger, Zipkin)

Also, we can further explore if this can be integrated into CI/CD pipeline ( ie running workflows on docker container instead of directly on github hosted virtual machines)

@lalitb lalitb requested a review from a team July 15, 2021 18:28
@codecov
Copy link

codecov bot commented Jul 15, 2021

Codecov Report

Merging #905 (5c4919b) into main (b1b1f64) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #905   +/-   ##
=======================================
  Coverage   93.39%   93.39%           
=======================================
  Files         165      165           
  Lines        6229     6229           
=======================================
  Hits         5817     5817           
  Misses        412      412           


#install thrift
ARG THRIFT_VERSION=0.14.1
RUN apt-get install -y --no-install-recommends \
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we invoke the setup_* scripts to create the container instead of duplicating the preparation steps?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes agree, this can be simplified. Will change it.

@github-actions github-actions bot added the Stale label Nov 11, 2021
@github-actions github-actions bot closed this Nov 19, 2021
@lalitb
Copy link
Member Author

lalitb commented Nov 19, 2021

incorrectly closed by bot.

@lalitb lalitb reopened this Nov 19, 2021
@github-actions github-actions bot removed the Stale label Nov 20, 2021
@ThomsonTan
Copy link
Contributor

Could we merge this as it is?

@lalitb
Copy link
Member Author

lalitb commented Nov 30, 2021

Could we merge this as it is?

I will fix it this week for comments, to be ready for merge.

@lalitb
Copy link
Member Author

lalitb commented Dec 1, 2021

Could we merge this as it is?

I will fix it this week for comments, to be ready for merge.

I think we are good to merge this. As discussed in the community meeting today, @esigo will be making further changes on this.

@ThomsonTan ThomsonTan merged commit 93249de into open-telemetry:main Dec 1, 2021
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

Successfully merging this pull request may close these issues.

2 participants