Add Zipkin exporter factory #207
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pjanotti
requested review from
bogdandrutu,
flands,
songy23 and
tigrannajaryan
as code owners
Codecov Report
@@ Coverage Diff @@
## master #207 +/- ##
==========================================
+ Coverage 72.31% 72.34% +0.02%
==========================================
Files 109 110 +1
Lines 6333 6347 +14
==========================================
+ Hits 4580 4592 +12
- Misses 1521 1523 +2
Partials 232 232
Continue to review full report at Codecov.
|
songy23
reviewed
Jul 30, 2019
| UploadPeriod *time.Duration `mapstructure:"upload_period,omitempty"` | ||
| ServiceName string `mapstructure:"service_name,omitempty"` | ||
| Endpoint string `mapstructure:"endpoint,omitempty"` | ||
| UploadPeriod *time.Duration `mapstructure:"upload_period,omitempty"` |
There was a problem hiding this comment.
Should this be added to new config?
There was a problem hiding this comment.
UploadPeriod is an option for the Zipkin exporter that i t should be handled in equivalent form via the pipeline, the same way that we do for OC (eg.: batcher, queued-retry).
ServiceName shouldn't actually be necessary, of course it still possible to have some bug in a client library not providing that information but it doesn't make much sense to inject it at the service as a configurable option, a place holder should suffice.
ccaraman
reviewed
Jul 30, 2019
There was a problem hiding this comment.
Could you also add some documentation for the zipkin exporter in https://github.com/open-telemetry/opentelemetry-service/blob/master/exporter/README.md?
|
@ccaraman I will do a separate PR for changes in the exporters README and will include the changes related to Zipkin. |
ccaraman
approved these changes
Jul 30, 2019
Merged
pjanotti
force-pushed
the
add-zipkin-exporter-factory
branch
from
6d39947
to
7a4f6a8
Compare
pjanotti
commented
Jul 31, 2019
songy23
approved these changes
Jul 31, 2019
added 5 commits
July 31, 2019 14:03
Add the Zipkin exporter configuration factory using the new configuration format. This does not bring many of the settings from the old configuration since they won't be used. In a sub-sequent PR the code of the exporter itself will be changed.
Replaced the TODO commented code with an issue.
pjanotti
force-pushed
the
add-zipkin-exporter-factory
branch
from
b7b4ffe
to
e49f7b8
Compare
|
Removed the commented code on the TODO and added a proper issue to track it: #216. |
tigrannajaryan
approved these changes
Aug 1, 2019
pjanotti
pushed a commit
that referenced
this pull request
Aug 7, 2019
* Migrate updates to Prometheus receiver from opencensus-service * Remove unnecessary configuration for adjusting metrics * Update exporters README (#210) Remove stale content, put in place a simple skeleton, and put information for the Zipkin exporter. * Added Jaeger gRPC receiver (#197) * Added Jaeger gRPC receiver Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de> * Addressed first review round Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de> * Changes based on the feedback from the second review Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de> * Fixed import order, check for same ip:port on endpoints Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de> * Reverted the port-check Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de> * Fix README - remove broken links and commands (#211) Signed-off-by: Hui Kang <kangh@us.ibm.com> * Refactor opencensus exporter to make it easily extensible (#212) Refactored oc exporter code to make it easier to import in derived builds without copying all the code. Example derived exporter: https://github.com/owais/example-derived-oc-exporter - Moved internal/compression to root - Split opencensusexporter factory's `CreateTraceExporter` method into - `OCAgentOptions` and `CreateOCAgent` * Use sha256 instead of md5 in node batcher processor (#202) MD5 is insecure cryptographic hash functions, and are therefore unsuitable for security applications. * Add goimports check and fix import order for all files (#209) * Add goimports check and fix import order for all files Updates #155. * Try specifying a version for goimports * Show the diff instead of files and fix import * Fix default in Makefile * Add factory and update config for tail sampling processor (#200) * Add factory and update config for tail sampling processor Updates #146 * Move to package processor * Remove an unnecessary check * Move CreateDefaultConfig to factory and add unit tests * Fix test failure * Remove commented code * Add misspell check and fix all typos (#214) * Add misspell check and fix all typos Updates #155. * Format * Include yaml files * Move tail sampling config to its own package and remove stale configs (#217) Follow up of #200. Second step of #146. * Add Observability Vision document (#188) * Add Observability Vision document This is a guidance document that developers can consult with when working on improving own observability of OpenTelemetry Service. * PR fixes * Add Zipkin exporter factory (#207) * Add Zipkin exporter factory Add the Zipkin exporter configuration factory using the new configuration format. This does not bring many of the settings from the old configuration since they won't be used. In a sub-sequent PR the code of the exporter itself will be changed. * PR Feedback: add 1 test plus some comments * Rename endpoint to http-url and related field * Fix goimports issue * Remove TODO commented code Replaced the TODO commented code with an issue. * Rename the field used to specify destination * Update README to address reviewer comments. * Update module dependencies and import order for metricfamily * Improve OC Receiver Documentation (#203) * First round of receiver and opencensus receiver documentation. * Undo go mod/sum changes * Address initial set of comments. * Address next set of comments. * Address next set of comments. * Fix use of server instead of receiver in comment and explain settings can be mix and matched. * Merged master and fixed mispell error caugh with new tools * Add static check and fix all errors (#218) * Add static check Fixes #155. * Fix most staticcheck errors * More fixes * Fix id_batcher * Rename exporterhelp parameter (#220) The paramater was named exportFormat but it actually used as the exporter name. * Fix build: lower casing error message (#224) * Add jaeger grpc exporter (#219) * Add Jaeger gRPC exporter Adds a Jaeger gRPC exporter. * Update exporters/README.md * Fixes on the exporter/README.md * Add doc.go with package description. * Fix import order on config_test.go * Migrate updates to Prometheus receiver from opencensus-service * Remove unnecessary configuration for adjusting metrics * Update README to address reviewer comments. * Update module dependencies and import order for metricfamily * Fix goimports * Fix staticcheck issues
hughesjj
pushed a commit
to hughesjj/opentelemetry-collector
that referenced
this pull request
Apr 27, 2023
Troels51
pushed a commit
to Troels51/opentelemetry-collector
that referenced
this pull request
Jul 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add the Zipkin exporter configuration factory using the new configuration format. This does not bring many of the settings from the old configuration since they won't be used. In a sub-sequent PR the code of the exporter itself will be changed.
Closes #34