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

update opentelemetry to 0.19.0 #3196

Merged
merged 15 commits into from
Jun 5, 2023
Merged

update opentelemetry to 0.19.0 #3196

merged 15 commits into from
Jun 5, 2023

Conversation

garypen
Copy link
Contributor

@garypen garypen commented May 31, 2023

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 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.

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

Checklist

Complete the checklist (and note appropriate exceptions) before a final PR is raised.

  • Changes are compatible[^1]
  • Documentation[^2] completed
  • Performance impact assessed and acceptable
  • Tests added and passing[^3]
    • Unit Tests
    • 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

I've commented out the call to a non-existent method (with_memory()),
which seems like all I can do right now. I'm not sure what the impact of
that is, so ...
@garypen garypen self-assigned this May 31, 2023
@github-actions

This comment has been minimized.

@router-perf
Copy link

router-perf bot commented May 31, 2023

CI performance tests

  • const - Basic stress test that runs with a constant number of users
  • no-graphos - Basic stress test, no GraphOS.
  • step - Basic stress test that steps up the number of users over time
  • reload - Reload test over a long period of time at a constant rate of users
  • xlarge-request - Stress test with 10 MB request payload
  • large-request - Stress test with a 1 MB request payload
  • xxlarge-request - Stress test with 100 MB request payload

@garypen garypen changed the title first compiling version update opentelemetry to 0.19.0 May 31, 2023
garypen added 3 commits June 1, 2023 08:54
 - add the new otel attributes to the prometheus tests
 - rename the match to include "_total" in the name

I'm not sure if that second change is correct. I think it is, because I
think the original behaviour (which appears to have changed) was a bit
looser and accepted a less specific name (looks like a synonym).

However, I'm not a prometheus file format expert, so I'll need to
verify that.
@garypen garypen marked this pull request as ready for review June 1, 2023 16:27
@garypen garypen requested review from a team, Geal, BrynCooke, bnjjj and SimonSapin and removed request for BrynCooke June 1, 2023 16:27
@tmc
Copy link

tmc commented Jun 4, 2023

would love to see this go in

@garypen garypen enabled auto-merge (squash) June 5, 2023 07:13
@garypen garypen merged commit 2ca2d73 into dev Jun 5, 2023
@garypen garypen deleted the garypen/2878-otel-0.19.0 branch June 5, 2023 07:58
o0Ignition0o added a commit that referenced this pull request Jun 15, 2023
o0Ignition0o added a commit that referenced this pull request Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants