-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add config to docker image #1792
Conversation
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
Makefile
Outdated
@@ -189,6 +189,7 @@ delete-tag: | |||
|
|||
.PHONY: docker-otelcol | |||
docker-otelcol: | |||
cp examples/otel-local-config.yaml cmd/otelcol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not nice but seems better than setting the docker context to the root of the project.
Codecov Report
@@ Coverage Diff @@
## master #1792 +/- ##
==========================================
- Coverage 92.38% 91.53% -0.85%
==========================================
Files 258 258
Lines 18201 15564 -2637
==========================================
- Hits 16815 14247 -2568
+ Misses 972 906 -66
+ Partials 414 411 -3
Continue to review full report at Codecov.
|
@pavolloffay this merely places the example config file in the docker image so that it is available for the user as a starting point for their custom config file, right? |
Correct see the example commands in the first comment on how the config can be edited (first copy to host then edit). Or users can run it directly with log exporters for quick demo purposes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Will wait a bit before mering to see @owais has any comments.
Contrib docker image uses
assuming there is a config file at Can we make the following changes?
ENTRYPOINT ["/otelcol"]
CMD ["--config", "/etc/otel/config.yaml"]
This would ship a default config with the collector while being consistent with the contrib image. We should probably have a default config file for contrib image as well once this is merged. |
I'm not sure about shipping the example config as the default one. I think the default config should be more or less usable out of the box and needs to be vetted more carefully compared to an example config file. Also an example config might contains a lot of stuff than we want to enable by defaults. So probably a separate dedicated default config file would be better (even if it's a 1:1 copy of example config today). |
@owais I have made the proposed changes to the dockerfile.
The default and example configs should be both ready to run out of the box. At the moment I don't see any difference between between example and default config. Both should use sane components and document what is being used for users to understand how to change the config. I have copied the example config to the otel dir for now. |
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but let's enable OTLP in the default config file.
pipelines: | ||
|
||
traces: | ||
receivers: [opencensus, jaeger, zipkin] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should enable OTLP by default. I think it should be the default or recommended receiver for all data types.
@owais I will submit a follow-up PR to enable OTLP in the example and default configs. |
…jaeger (open-telemetry#1792) * Bump google.golang.org/api in /exporters/trace/jaeger Bumps [google.golang.org/api](https://github.com/googleapis/google-api-go-client) from 0.43.0 to 0.44.0. - [Release notes](https://github.com/googleapis/google-api-go-client/releases) - [Changelog](https://github.com/googleapis/google-api-go-client/blob/master/CHANGES.md) - [Commits](googleapis/google-api-go-client@v0.43.0...v0.44.0) Signed-off-by: dependabot[bot] <support@github.com> * Auto-fix go.sum changes in dependent modules Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
…y#1792) Bumps [boto3](https://github.com/boto/boto3) from 1.24.32 to 1.24.33. - [Release notes](https://github.com/boto/boto3/releases) - [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst) - [Commits](boto/boto3@1.24.32...1.24.33) --- updated-dependencies: - dependency-name: boto3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Pavol Loffay ploffay@redhat.com
Description: <Describe what has changed.
Add
examples/otel-local-config.yaml
into docker image to make it easier for users to get a default config.@tigrannajaryan could you please have a look?
Link to tracking Issue:
Updates #886
Follow up PR should add config into other distributions and add comments into the config.
Testing: < Describe what testing was performed and which tests were added.>
Tested docker build locally.
Documentation: < Describe the documentation added.>
Once this is merged I will update Docker instructions https://opentelemetry.io/docs/collector/about/ to use