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

feat(telemetry): add support of custom events in telemetry #4956

Merged
merged 24 commits into from
Apr 22, 2024
Merged

feat(telemetry): add support of custom events in telemetry #4956

merged 24 commits into from
Apr 22, 2024

Conversation

bnjjj
Copy link
Contributor

@bnjjj bnjjj commented Apr 15, 2024 •
edited

Fixes #4320

Checklist

Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.

  • Changes are compatible1
  • Documentation2 completed
  • Performance impact assessed and acceptable
  • Tests added and passing3
    • Unit Tests
    • Integration Tests
    • Manual Tests

Exceptions

Note any exceptions here

Notes

Footnotes

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

@bnjjj
wip
fb2d0b6 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
add support of events
ae546b8 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
add tests for events
62d400d 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
fix test
28d93cd 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
fix test and ordering of attributes and headers
5bc0287 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
fix lint
2e56947 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
Merge branch 'dev' of github.com:apollographql/router into bnjjj/feat…
4ac8833 
…_4320
@bnjjj
delete useless commment
8ef1a15 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@github-actions GitHub Actions
Copy link
Contributor

@bnjjj, please consider creating a changeset entry in /.changesets/. These instructions describe the process and tooling.

@bnjjj bnjjj requested review from BrynCooke and Geal April 15, 2024 12:25
bnjjj added 11 commits April 15, 2024 14:30
@bnjjj
update metrics
9bba758 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
wip
3235c57 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
wip
c3151bf 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
refactor event attributes
3f73f8b 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
tests wip
91402d7 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
Merge branch 'dev' of github.com:apollographql/router into bnjjj/feat…
d3d7740 
…_4320
@bnjjj
fix custom event detection in otel
81f7e87 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
add tests
6a82c3e 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
improve handling of object as an otel attribute
c83c6d9 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
fix snapshot
b513b51 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
Merge branch 'bnjjj/feat_4320' of github.com:apollographql/router int…
84d3655 
…o bnjjj/feat_4320
Geal
Geal approved these changes Apr 18, 2024
View reviewed changes
Copy link
Contributor

@Geal Geal left a comment

Choose a reason for hiding this comment

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

this looks very useful! We should make sure that we do not steer users too easily towards the full request or response dumps because they will be very expensive.
Could the subgraph events be activated per subgraph service name too? Instead of only on all subgraphs

apollo-router/src/lib.rs Outdated Show resolved Hide resolved
apollo-router/src/plugins/telemetry/config_new/events.rs Show resolved Hide resolved
@bnjjj
add more tests for events
7c41055 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
fix otel value test
6d0c232 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
Merge branch 'dev' of github.com:apollographql/router into bnjjj/feat…
60ca42e 
…_4320
@bnjjj
Copy link
Contributor Author

bnjjj commented Apr 18, 2024

@Geal Yes we will need to add these disclaimer in our docs. I think @shorgi will help me to find the right way to achieve that.

I also created an issue for your suggestion #4984 @Geal

@bnjjj
fixes
080ef4c 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
fix lint
c9ce9bd 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
BrynCooke
BrynCooke approved these changes Apr 19, 2024
View reviewed changes
Copy link
Contributor

@BrynCooke BrynCooke left a comment

Choose a reason for hiding this comment

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

I played around with this quite a bit. It's great!

@BrynCooke BrynCooke merged commit 2f1ef5d into dev Apr 22, 2024
14 checks passed
@BrynCooke BrynCooke deleted the bnjjj/feat_4320 branch April 22, 2024 08:01
@shorgi shorgi mentioned this pull request Apr 22, 2024
Merged
bnjjj pushed a commit that referenced this pull request Apr 23, 2024
@shorgi
docs: custom events telemetry (#5001)
8d39abf 
Docs for #4320 (PR
#4956):

- Unhide Events page and add to left-nav
- Add telemetry performance trade-off note
@bnjjj bnjjj mentioned this pull request Apr 29, 2024
Merged
bnjjj added a commit that referenced this pull request Apr 29, 2024
@bnjjj @BrynCooke
add 2 changeset for telemetry (#5039)
ae0d45e 
Add changelog entry for
#4956 and
#4987

---------

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Co-authored-by: Bryn Cooke <BrynCooke@gmail.com>
@BrynCooke BrynCooke mentioned this pull request May 7, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Geal Geal Geal approved these changes

@BrynCooke BrynCooke BrynCooke approved these changes

Assignees

@bnjjj bnjjj

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Custom events defined by yaml
3 participants
@bnjjj @Geal @BrynCooke