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.

Sign up for GitHub

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

Jump to bottom

OpenTelemetry service name should have higher priority than app name when no resource attributes are set #35193

Merged
merged 1 commit into from
Aug 7, 2023
Merged

OpenTelemetry service name should have higher priority than app name when no resource attributes are set #35193

merged 1 commit into from
Aug 7, 2023

Conversation

michalvavrik
Copy link
Member

@michalvavrik michalvavrik commented Aug 3, 2023 •
edited by geoand
Loading

Fixes: #33317

@michalvavrik michalvavrik requested a review from brunobat August 3, 2023 20:14
@quarkus-bot
Copy link

quarkus-bot bot commented Aug 3, 2023

/cc @brunobat (opentelemetry), @radcortez (opentelemetry)

@michalvavrik michalvavrik mentioned this pull request Aug 3, 2023
Closed
@quarkus-bot

This comment has been minimized.

Sign in to view
@michalvavrik michalvavrik force-pushed the feature/fix-otel-svc-name-when-no-attrs branch 2 times, most recently from 78cd5b7 to 8a8ae3f Compare August 4, 2023 09:44
@michalvavrik michalvavrik requested a review from brunobat August 4, 2023 09:47
@quarkus-bot

This comment has been minimized.

Sign in to view
@brunobat
Copy link
Contributor

brunobat commented Aug 4, 2023

@michalvavrik I'm creating a set of tests, if you don't mind, I'll push them to your PR

@michalvavrik
Copy link
Member Author

@michalvavrik I'm creating a set of tests, if you don't mind, I'll push them to your PR

Please go ahead, whatever feels right for you!

@brunobat
Copy link
Contributor

brunobat commented Aug 4, 2023

Test io.quarkus.opentelemetry.deployment.OpenTelemetryServiceNameAppNameTest is failing.

@michalvavrik
Copy link
Member Author

Test io.quarkus.opentelemetry.deployment.OpenTelemetryServiceNameAppNameTest is failing.

as it should, it is build time property

@brunobat
Copy link
Contributor

brunobat commented Aug 4, 2023

Test io.quarkus.opentelemetry.deployment.OpenTelemetryServiceNameAppNameTest is failing.

as it should, it is build time property

It is Runtime since 3.0:

quarkus/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/config/runtime/OTelRuntimeConfig.java

Line 49 in 0d3cb35

Optional<List<String>> resourceAttributes();

@michalvavrik
Copy link
Member Author

Test io.quarkus.opentelemetry.deployment.OpenTelemetryServiceNameAppNameTest is failing.

as it should, it is build time property

It is Runtime since 3.0:

quarkus/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/config/runtime/OTelRuntimeConfig.java

Line 49 in 0d3cb35

Optional<List<String>> resourceAttributes();

I was referring to the

quarkus/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetryServiceNameAppNameTest.java

Line 21 in 04ead57

.overrideRuntimeConfigKey("quarkus.application.name", SERVICE_NAME);
which can't be done and therefore, it has no effect and the test fails.

@quarkus-bot

This comment has been minimized.

Sign in to view
brunobat
brunobat approved these changes Aug 4, 2023
View reviewed changes
Copy link
Contributor

@brunobat brunobat left a comment

Choose a reason for hiding this comment

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

Should be fixed now. Feel free to squash my commits.

@michalvavrik michalvavrik force-pushed the feature/fix-otel-svc-name-when-no-attrs branch from b3fc8b5 to 83cbb81 Compare August 4, 2023 13:47
@michalvavrik michalvavrik force-pushed the feature/fix-otel-svc-name-when-no-attrs branch from 83cbb81 to c36cb73 Compare August 4, 2023 13:51
@michalvavrik
Copy link
Member Author

Should be fixed now. Feel free to squash my commits.

Done, I also changed .overrideRuntimeConfigKey("quarkus.application.name", "application-name-must-fail") to .overrideConfigKey("quarkus.application.name", "application-name-must-fail").

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 4, 2023

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

@geoand geoand merged commit 0a84b4e into quarkusio:main Aug 7, 2023
@quarkus-bot quarkus-bot bot added this to the 3.3 - main milestone Aug 7, 2023
@quarkus-bot

This comment was marked as resolved.

Sign in to view
@geoand
Copy link
Contributor

geoand commented Aug 7, 2023

Should #33317 now we be closed again?

@michalvavrik michalvavrik deleted the feature/fix-otel-svc-name-when-no-attrs branch August 7, 2023 13:14
@michalvavrik
Copy link
Member Author

Should #33317 now we be closed again?

Yep

@gsmet gsmet modified the milestones: 3.3.0.CR1, 3.2.4.Final Aug 9, 2023
benkard pushed a commit to benkard/mulkcms2 that referenced this pull request Aug 29, 2023
Update all non-major dependencies (mulk/mulkcms2!32)
2f015c7 
This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [flow-bin](https://github.com/flowtype/flow-bin) ([changelog](https://github.com/facebook/flow/blob/master/Changelog.md)) | devDependencies | minor | [`^0.214.0` -> `^0.215.0`](https://renovatebot.com/diffs/npm/flow-bin/0.214.0/0.215.1) |
| [org.liquibase:liquibase-maven-plugin](http://www.liquibase.org/liquibase-maven-plugin) ([source](https://github.com/liquibase/liquibase)) | build | patch | `4.23.0` -> `4.23.1` |
| [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | minor | `3.2.3.Final` -> `3.3.0` |
| [io.quarkus:quarkus-universe-bom](https://github.com/quarkusio/quarkus-platform) | import | minor | `3.2.3.Final` -> `3.3.0` |
| [org.apache.maven.plugins:maven-enforcer-plugin](https://maven.apache.org/enforcer/) | build | minor | `3.3.0` -> `3.4.0` |

---

### Release Notes

<details>
<summary>flowtype/flow-bin</summary>

### [`v0.215.1`](flow/flow-bin@a92ce80...cbb038f)

[Compare Source](flow/flow-bin@a92ce80...cbb038f)

### [`v0.215.0`](flow/flow-bin@ca11e28...a92ce80)

[Compare Source](flow/flow-bin@ca11e28...a92ce80)

</details>

<details>
<summary>liquibase/liquibase</summary>

### [`v4.23.1`](https://github.com/liquibase/liquibase/blob/HEAD/changelog.txt#Liquibase-4231-is-a-patch-release)

[Compare Source](liquibase/liquibase@v4.23.0...v4.23.1)

</details>

<details>
<summary>quarkusio/quarkus</summary>

### [`v3.3.0`](https://github.com/quarkusio/quarkus/releases/tag/3.3.0)

[Compare Source](quarkusio/quarkus@3.2.4.Final...3.3.0)

##### Complete changelog

-   [#&#8203;35350](quarkusio/quarkus#35350) - Fix package type system property clearing
-   [#&#8203;35348](quarkusio/quarkus#35348) - quarkus-maven-plugin runs native building even if the profile is commented out
-   [#&#8203;35343](quarkusio/quarkus#35343) - ArC: fix StackOverflowError in AutoAddScopeBuildItem
-   [#&#8203;35319](quarkusio/quarkus#35319) - Register arrays of Hibernate ORM's JDBC basic types for reflection
-   [#&#8203;35315](quarkusio/quarkus#35315) - Fix Datasource timing issues with Liquibase / Flyway and OpenTelemetry
-   [#&#8203;35314](quarkusio/quarkus#35314) - Regression in 3.3.0.CR1: Synthetic bean instance for io.opentelemetry.api.OpenTelemetry not initialized yet
-   [#&#8203;35312](quarkusio/quarkus#35312) - Updates Infinispan to 14.0.13.Final
-   [#&#8203;35308](quarkusio/quarkus#35308) - Lock jib execution to avoid OverlappingFileLockException in parallel builds
-   [#&#8203;35305](quarkusio/quarkus#35305) - Fix the titles of the tables in RESTEasy Reactive doc
-   [#&#8203;35302](quarkusio/quarkus#35302) - Docs: Mention wilcard support in resteasy reactive XML serialisation exclude classes configuration
-   [#&#8203;35301](quarkusio/quarkus#35301) - Fix potential NPE in quarkus-csrf-reactive when no MediaType is found
-   [#&#8203;35299](quarkusio/quarkus#35299) - Output build graph using `quarkus.builder.graph-output` property
-   [#&#8203;35285](quarkusio/quarkus#35285) - NullPointerException during http post request when quarkus-csrf-reactive extension is added to a project
-   [#&#8203;35283](quarkusio/quarkus#35283) - Upgrade proto-google-common-protos to 2.23.0
-   [#&#8203;35282](quarkusio/quarkus#35282) - Avoid keeping references to BytecodeRecorderImpl
-   [#&#8203;35276](quarkusio/quarkus#35276) - Reinstate DevModeTestUtil to avoid breaking other projects that depend on it
-   [#&#8203;35273](quarkusio/quarkus#35273) - Fix small typo in comment
-   [#&#8203;35263](quarkusio/quarkus#35263) - Stop the recovery service while ArC is still around
-   [#&#8203;35245](quarkusio/quarkus#35245) - Add missing info to init Jobs
-   [#&#8203;35244](quarkusio/quarkus#35244) - Init Jobs are missing ServiceAccount and Image Pull Secrets
-   [#&#8203;35240](quarkusio/quarkus#35240) - Update SmallRye Health to 4.0.4
-   [#&#8203;34071](quarkusio/quarkus#34071) - 3.1.1 Final - java.lang.IllegalArgumentException: Class java.util.UUID\[] is instantiated reflectively but was never registered
-   [#&#8203;32800](quarkusio/quarkus#32800) - Duplicated checks in health check response
-   [#&#8203;11903](quarkusio/quarkus#11903) - Gradle multimodule project + quarkus-container-image-jib: OverlappingFileLockException

### [`v3.2.4.Final`](https://github.com/quarkusio/quarkus/releases/tag/3.2.4.Final)

[Compare Source](quarkusio/quarkus@3.2.3.Final...3.2.4.Final)

##### Complete changelog

-   [#&#8203;35300](quarkusio/quarkus#35300) - Fix `jandex-gradle-plugin-version` in CDI Reference
-   [#&#8203;35296](quarkusio/quarkus#35296) - Upgrade H2 to 2.2.220
-   [#&#8203;35258](quarkusio/quarkus#35258) - CDI Reference 1.1 has incomplete information for gradle
-   [#&#8203;35255](quarkusio/quarkus#35255) - Quartz: QuarkusMSSQLDelegate should extends MSSQLDelegate
-   [#&#8203;35211](quarkusio/quarkus#35211) - Document Maven config options that may be relevant when running tests
-   [#&#8203;35203](quarkusio/quarkus#35203) - Pass Maven user settings when initializing artifact resolver
-   [#&#8203;35193](quarkusio/quarkus#35193) - OpenTelemetry service name should have higher priority than app name when no resource attributes are set
-   [#&#8203;35189](quarkusio/quarkus#35189) - Quarkus CLI fixes
-   [#&#8203;35188](quarkusio/quarkus#35188) - SmallRyeGraphQLOverWebSocketHandler: use order value > Integer.MIN_VALUE
-   [#&#8203;35181](quarkusio/quarkus#35181) - REST Data with Panache should not produce links when hal is disabled
-   [#&#8203;35174](quarkusio/quarkus#35174) - Ensure the narayana-jta extension fully shuts down the recovery manager
-   [#&#8203;35172](quarkusio/quarkus#35172) - Kafka Streams: restore the feature name at Quarkus startup
-   [#&#8203;35171](quarkusio/quarkus#35171) - kafka-streams: feature not listed on startup
-   [#&#8203;35165](quarkusio/quarkus#35165) - Propagate all user methods in REST Data with Panache
-   [#&#8203;35160](quarkusio/quarkus#35160) - Properly use internal links to point to other guides
-   [#&#8203;35140](quarkusio/quarkus#35140) - ArC: fix deadlock when calling guarded methods on the same thread
-   [#&#8203;35136](quarkusio/quarkus#35136) - Deadlock while calling write-locked method from read-locked method
-   [#&#8203;34908](quarkusio/quarkus#34908) - `@RouteFilter` stopped working with WebSocket requests Quarkus 3.2.0.Final
-   [#&#8203;34875](quarkusio/quarkus#34875) - Quarkus build does not work since 3.2.0 with teamcity/plexus launcher
-   [#&#8203;34713](quarkusio/quarkus#34713) - Option to track build configuration for changes between builds
-   [#&#8203;34576](quarkusio/quarkus#34576) - Live reload stopped working on 3.2 when using XA transactions
-   [#&#8203;34514](quarkusio/quarkus#34514) - Support `@WithUnnamedKey` in documentation
-   [#&#8203;34065](quarkusio/quarkus#34065) - Add support for project Java version update based on extensions
-   [#&#8203;33317](quarkusio/quarkus#33317) - OpenTelemetry SDK autoconfiguration ignores OTEL service name in favor of Quarkus app name
-   [#&#8203;15461](quarkusio/quarkus#15461) - Quarkus tests fails mTLS authentication against internal Maven repository

</details>

<details>
<summary>quarkusio/quarkus-platform</summary>

### [`v3.3.0`](quarkusio/quarkus-platform@3.2.4.Final...3.3.0)

[Compare Source](quarkusio/quarkus-platform@3.2.4.Final...3.3.0)

### [`v3.2.4.Final`](quarkusio/quarkus-platform@3.2.3.Final...3.2.4.Final)

[Compare Source](quarkusio/quarkus-platform@3.2.3.Final...3.2.4.Final)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

👻 **Immortal**: This MR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired.

---

 - [x] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
benkard pushed a commit to benkard/quarkus-googlecloud-jsonlogging that referenced this pull request Sep 21, 2023
chore(deps): update all non-major dependencies to v3.3.0 (mulk/quarku…
fb32242 
…s-googlecloud-jsonlogging!17)

This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [io.quarkus:quarkus-extension-processor](https://github.com/quarkusio/quarkus) |  | minor | `3.2.3.Final` -> `3.3.0` |
| [io.quarkus:quarkus-extension-maven-plugin](https://github.com/quarkusio/quarkus) | build | minor | `3.2.3.Final` -> `3.3.0` |
| [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | minor | `3.2.3.Final` -> `3.3.0` |
| [io.quarkus:quarkus-bom](https://github.com/quarkusio/quarkus) | import | minor | `3.2.3.Final` -> `3.3.0` |

---

### Release Notes

<details>
<summary>quarkusio/quarkus</summary>

### [`v3.3.0`](quarkusio/quarkus@3.2.4.Final...3.3.0)

[Compare Source](quarkusio/quarkus@3.2.4.Final...3.3.0)

### [`v3.2.4.Final`](https://github.com/quarkusio/quarkus/releases/tag/3.2.4.Final)

[Compare Source](quarkusio/quarkus@3.2.3.Final...3.2.4.Final)

##### Complete changelog

-   [#&#8203;35300](quarkusio/quarkus#35300) - Fix `jandex-gradle-plugin-version` in CDI Reference
-   [#&#8203;35296](quarkusio/quarkus#35296) - Upgrade H2 to 2.2.220
-   [#&#8203;35258](quarkusio/quarkus#35258) - CDI Reference 1.1 has incomplete information for gradle
-   [#&#8203;35255](quarkusio/quarkus#35255) - Quartz: QuarkusMSSQLDelegate should extends MSSQLDelegate
-   [#&#8203;35211](quarkusio/quarkus#35211) - Document Maven config options that may be relevant when running tests
-   [#&#8203;35203](quarkusio/quarkus#35203) - Pass Maven user settings when initializing artifact resolver
-   [#&#8203;35193](quarkusio/quarkus#35193) - OpenTelemetry service name should have higher priority than app name when no resource attributes are set
-   [#&#8203;35189](quarkusio/quarkus#35189) - Quarkus CLI fixes
-   [#&#8203;35188](quarkusio/quarkus#35188) - SmallRyeGraphQLOverWebSocketHandler: use order value > Integer.MIN_VALUE
-   [#&#8203;35181](quarkusio/quarkus#35181) - REST Data with Panache should not produce links when hal is disabled
-   [#&#8203;35174](quarkusio/quarkus#35174) - Ensure the narayana-jta extension fully shuts down the recovery manager
-   [#&#8203;35172](quarkusio/quarkus#35172) - Kafka Streams: restore the feature name at Quarkus startup
-   [#&#8203;35171](quarkusio/quarkus#35171) - kafka-streams: feature not listed on startup
-   [#&#8203;35165](quarkusio/quarkus#35165) - Propagate all user methods in REST Data with Panache
-   [#&#8203;35160](quarkusio/quarkus#35160) - Properly use internal links to point to other guides
-   [#&#8203;35140](quarkusio/quarkus#35140) - ArC: fix deadlock when calling guarded methods on the same thread
-   [#&#8203;35136](quarkusio/quarkus#35136) - Deadlock while calling write-locked method from read-locked method
-   [#&#8203;34908](quarkusio/quarkus#34908) - `@RouteFilter` stopped working with WebSocket requests Quarkus 3.2.0.Final
-   [#&#8203;34875](quarkusio/quarkus#34875) - Quarkus build does not work since 3.2.0 with teamcity/plexus launcher
-   [#&#8203;34713](quarkusio/quarkus#34713) - Option to track build configuration for changes between builds
-   [#&#8203;34576](quarkusio/quarkus#34576) - Live reload stopped working on 3.2 when using XA transactions
-   [#&#8203;34514](quarkusio/quarkus#34514) - Support `@WithUnnamedKey` in documentation
-   [#&#8203;34065](quarkusio/quarkus#34065) - Add support for project Java version update based on extensions
-   [#&#8203;33317](quarkusio/quarkus#33317) - OpenTelemetry SDK autoconfiguration ignores OTEL service name in favor of Quarkus app name
-   [#&#8203;15461](quarkusio/quarkus#15461) - Quarkus tests fails mTLS authentication against internal Maven repository

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@radcortez radcortez radcortez approved these changes

@brunobat brunobat brunobat approved these changes

Assignees
No one assigned
Labels
area/tracing kind/bugfix
Projects
None yet
Milestone
3.2.4.Final
Development

Successfully merging this pull request may close these issues.

OpenTelemetry SDK autoconfiguration ignores OTEL service name in favor of Quarkus app name
5 participants
@michalvavrik @brunobat @geoand @radcortez @gsmet