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

Dont forcefully set service_name option on any exporter #43

Merged
merged 1 commit into from
Feb 21, 2022

Conversation

cedricziel
Copy link
Contributor

Some exporters do not define this option (OTLP f.e.) and setting this option
breaks them.

Some exporters do not define this option (OTLP f.e.) and setting this option
breaks them.
@tidal
Copy link
Member

tidal commented Feb 15, 2022

@cedricziel
Thanks for your contribution and sorry again for the late response.
As I mentioned on slack, we are busy setting up the infrastructure to have better 2e2 test at the moment.

There is one integration test failing due to your change.
I know the integration tests are not an easy read at the moment, so here is how you can fix the test.
I believe removing the 'service_name' => 'foo', entry should do the trick.
https://github.com/open-telemetry/opentelemetry-php-contrib/blob/main/tests/Integration/Symfony/OtelSdkBundle/DependencyInjection/OtelSdkExtensionTest.php#L338

The other way would be to add a serviceName to the MockExporter -> https://github.com/open-telemetry/opentelemetry-php-contrib/blob/main/tests/Integration/Symfony/OtelSdkBundle/Mock/SpanExporter.php
However I'm not really sure atm, if that would break any other tests.

@tidal
Copy link
Member

tidal commented Feb 21, 2022

Merging this, since the tests should be fixed with: #47

@tidal tidal merged commit 5cbb37f into open-telemetry:main Feb 21, 2022
@cedricziel cedricziel deleted the patch-1 branch February 21, 2022 21:50
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