Use router path as resource name

[inikolaev]

Fixes #27

[florimondmanca]

Codecov Report

Merging #28 into master will decrease coverage by 94.07%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##            master     #28       +/-   ##
===========================================
- Coverage   100.00%   5.92%   -94.08%     
===========================================
  Files            9       6        -3     
  Lines          348     253       -95     
===========================================
- Hits           348      15      -333     
- Misses           0     238      +238     

Impacted Files	Coverage Δ
tests/applications/raw.py	0.00% <0.00%> (-100.00%)	⬇️
tests/applications/fastapi.py	0.00% <0.00%> (-100.00%)	⬇️
tests/applications/starlette.py	0.00% <0.00%> (-100.00%)	⬇️
tests/test_trace_middleware.py	6.31% <0.00%> (-93.69%)	⬇️
tests/utils/asgi.py
tests/utils/config.py

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32ccb56...d4b61e7. Read the comment docs.

[codecov-io]

Codecov Report

Merging #28 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #28   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           9      9           
  Lines         354    434   +80     
=====================================
+ Hits          354    434   +80

Impacted Files	Coverage Δ
tests/test_trace_middleware.py	100% <100%> (ø)	⬆️
tests/applications/starlette.py	100% <100%> (ø)	⬆️
tests/applications/raw.py	100% <100%> (ø)	⬆️
tests/applications/fastapi.py	100% <100%> (ø)	⬆️
src/ddtrace_asgi/middleware.py	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c6bd4c...3e77c65. Read the comment docs.

[inikolaev]

Can't figure out how to test partial match yet, which causes build to fail due to lower than expected test coverage.

[florimondmanca]

Thanks! This is lookind great so far.

Can't figure out how to test partial match yet

According to this snippet, Match.PARTIAL happens when the URL matches but the requested HTTP method isn't allowed by the endpoint:

https://github.com/encode/starlette/blob/508ab752b4f9fb4f5c414eec56f204459297825d/starlette/routing.py#L200-L201

So I assume we should be able to cover that code path by issuing a POST to the path_parameters endpoint?

[florimondmanca]

@inikolaev I pushed and merged two PRs (#29, #30) to clean up the test setup, feel free to merge master back in here — you should have a few issues off your plate. :-)

[inikolaev]

Thanks! This is lookind great so far.

Can't figure out how to test partial match yet

According to this snippet, Match.PARTIAL happens when the URL matches but the requested HTTP method isn't allowed by the endpoint:

https://github.com/encode/starlette/blob/508ab752b4f9fb4f5c414eec56f204459297825d/starlette/routing.py#L200-L201

So I assume we should be able to cover that code path by issuing a POST to the path_parameters endpoint?

Yes, I was just confused about which status code to expect, but now it should be fine.

[inikolaev]

I made enrich_span private for now, this would allow to change name/signature before it's made public.

[florimondmanca]

Thanks! I’ll try and leave a review soon. :)

[inikolaev]

Did you have a chance to look at it?

[florimondmanca]

Hi @inikolaev, I had forgotten about this so no, not yet. I keep the notification in my inbox and plan to address this week. :)

[florimondmanca]

Hi @inikolaev, having reviewed this, thought about this a bit more, and found more use cases for customizing the default behavior (eg for my personal website), I don't think this would be the right way to go anymore.

I opened #32 with ideas about an extension mechanism that should also allow you to resolve your use case (although it would indeed require writing some code). Can you take a look over there and confirm? Thanks! I'm closing this in the meantime, thanks for the time you've put into this. :-)

[inikolaev]

I’ll take a look!

Can you take a look at this PR about adding routes to Starlette scope and add some suggestions to make it more framework agnostic, since there’s nothing about routes in the standard: https://github.com/encode/starlette/pull/804/files

[florimondmanca]

@inikolaev I don't think it can be made framework agnostic, unfortunately.

The ASGI spec doesn't say anyway about how applications should handle routing (or whether they even have to). I played with the idea of a standardized routing ASGI extension, but we'd run across the same problem of "but what if my specific framework wants to add some more information?".

So I think encode/starlette#804 looks good right now. If nothing else if it should allow you to integrate with ddtrace-asgi more easily by inspecting the list of matched routes in scope. :-) Though that would need to happen after route matching happened, so after the app was called into… Might need to add one more hook to #32 as it currently doesn't support modifying the span after the response was sent.

[inikolaev]

That true, if frameworks want to add some framework-specific fields, that should be put into some standardized namespace, to avoid clashing with standard fields added in the future.