-
Notifications
You must be signed in to change notification settings - Fork 275
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
The update requires a change to the implementation and test update as follows: - In otel 0.18.0, processor factories had a `with_memory(bool)` method which we were using when building our prometheus exporter. AFAICT, this used to be a mechanism for controlling how metrics handled stale gauges. In 0.19.0, [this method was removed](open-telemetry/opentelemetry-rust#946) and now gauges are all assumed to be as though they were created with `false`. We had been providing `true` on our call. I'm not 100% certain of the impact of this change, but it appears that we can ignore it. We may need to consider it more carefully if problems arise. - There are now two standard OTEL attributes: ```otel_scope_name="apollo/router",otel_scope_version=""``` added to output and a number of tests had to be updated to accommodate that change. - One of our tests appeared to be searching for `apollo_router_cache_hit_count` (and this was working) when it should have been searching for `apollo_router_cache_hit_count_total` (likewise for miss). I've updated the test and think this is the correct thing to do. It looks like a bug was fixed in otel and this change matches the fix. Regarding that last point. The prometheus spec mandates naming format and the change was part of the compliance with that spec. This PR made the change: open-telemetry/opentelemetry-rust#952 The two affected counters in the router were: apollo_router_cache_hit_count -> apollo_router_cache_hit_count_total apollo_router_cache_miss_count -> apollo_router_cache_miss_count_total It's good that our prometheus metrics are now spec compliant, but we should note this in the release notes and (if possible) somewhere in our documentation. I'll add it to the changeset at least. The upgrade fixes many of the outstanding issues related to opentelemetry and various APM vendors: Fixes: #2878 Fixes: #2066 Fixes: #2959 Fixes: #2225 Fixes: #1520 <!-- start metadata --> **Checklist** Complete the checklist (and note appropriate exceptions) before a final PR is raised. - [x] Changes are compatible[^1] - [x] Documentation[^2] completed - [x] Performance impact assessed and acceptable - Tests added and passing[^3] - [x] Unit Tests - [x] Integration Tests - [ ] Manual Tests **Exceptions** *Note any exceptions here* **Notes** [^1]. It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. [^2]. Configuration is an important part of many changes. Where applicable please try to document configuration examples. [^3]. Tick whichever testing boxes are applicable. If you are adding Manual Tests: - please document the manual testing (extensively) in the Exceptions. - please raise a separate issue to automate the test and label it (or ask for it to be labeled) as `manual test`
- Loading branch information
Showing
8 changed files
with
110 additions
and
75 deletions.
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| ### update opentelemetry to 0.19.0 ([Issue #2878](https://github.com/apollographql/router/issues/2878)) | ||
|
|
||
|
|
||
| We've updated the following opentelemetry related crates: | ||
|
|
||
| ``` | ||
| opentelemetry 0.18.0 -> 0.19.0 | ||
| opentelemetry-datadog 0.6.0 -> 0.7.0 | ||
| opentelemetry-http 0.7.0 -> 0.8.0 | ||
| opentelemetry-jaeger 0.17.0 -> 0.18.0 | ||
| opentelemetry-otlp 0.11.0 -> 0.12.0 | ||
| opentelemetry-semantic-conventions 0.10.0 -> 0.11.0 | ||
| opentelemetry-zipkin 0.16.0 -> 0.17.0 | ||
| opentelemetry-prometheus 0.11.0 -> 0.12.0 | ||
| tracing-opentelemetry 0.18.0 -> 0.19.0 | ||
| ``` | ||
|
|
||
| This allows us to close a number of opentelemetry related issues. | ||
|
|
||
| Note: | ||
|
|
||
| The prometheus specification mandates naming format and, unfortunately, the router had two metrics which weren't compliant. The otel upgrade enforces the specification, so the affected metrics are now renamed (see below). | ||
|
|
||
| The two affected metrics in the router were: | ||
|
|
||
| apollo_router_cache_hit_count -> apollo_router_cache_hit_count_total | ||
| apollo_router_cache_miss_count -> apollo_router_cache_miss_count_total | ||
|
|
||
| If you are monitoring these metrics via prometheus, please update your dashboards with this name change. | ||
|
|
||
| By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/3421 |
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
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
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
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
Oops, something went wrong.