get matched route from context #1390

lftk · 2018-06-12T04:51:11Z

Get matched route from context to count the number of calls to the interface or to control user permissions.

codecov · 2018-06-12T04:57:31Z

Codecov Report

Merging #1390 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1390      +/-   ##
==========================================
+ Coverage   98.74%   98.74%   +<.01%     
==========================================
  Files          38       38              
  Lines        2144     2151       +7     
==========================================
+ Hits         2117     2124       +7     
  Misses         15       15              
  Partials       12       12

Impacted Files	Coverage Δ
context.go	`98.36% <100%> (ø)`	⬆️
tree.go	`100% <100%> (ø)`	⬆️
gin.go	`99.11% <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 5a7e309...7817c01. Read the comment docs.

jlordiales · 2018-07-12T12:10:41Z

any updates on this? Would be super useful to have (particularly for middlewares in my case)

nathancastelein · 2018-08-24T14:47:13Z

would love to have update on this, as this will be helpful in my code :)

drsect0r · 2018-09-12T09:27:32Z

This PR seems similar to pull request #1447.

drsect0r · 2018-09-15T13:11:55Z

@appleboy / @thinkerou what would be required to get the ball rolling on this pull request? Would love to see this change in production, just interested if I can perhaps run benchmarks or other work to speed things up here.

thinkerou · 2018-10-23T14:19:02Z

@youngblood @isgj @drsect0r please help us review the pr, thanks!
And hope that have comment how to use the feature and add this to readme.md, thanks!
And add some integration test case on gin_test.go.

thinkerou · 2018-10-23T14:29:22Z

ref #584 #649 #1447

tree_test.go

isgj · 2018-10-23T14:38:48Z

this PR is almost the same as #1447. The only difference is that this PR is calculating the route every time it scans the tree to find the handler. The #1447 stores the route in a node and doesn't have to calculate it.

Add test to check if the route is in the context

drsect0r · 2018-11-01T15:52:51Z

@4396 Thank you! @thinkerou would you have time to review?

chenshuijin · 2018-11-06T07:58:51Z

so, I would like to know when will this pr will be merged ? it's really useful for me.

appleboy

LGTM

Jim-Lambert-Bose · 2018-12-05T12:49:13Z

Any idea when these changes might get merged? I could really use this for my middleware. Thanks.

thinkerou

benchmark have some change. hold.

thinkerou · 2018-12-06T01:32:25Z

@4396 @drsect0r @isgj @Jim-Lambert-Bose

The master branch benchmark data:

➜  go-http-routing-benchmark git:(master) go test -bench=Gin
#GithubAPI Routes: 203
   Gin: 52064 Bytes

#GPlusAPI Routes: 13
   Gin: 3936 Bytes

#ParseAPI Routes: 26
   Gin: 6912 Bytes

#Static Routes: 157
   Gin: 30480 Bytes

goos: darwin
goarch: amd64
pkg: github.com/julienschmidt/go-http-routing-benchmark
BenchmarkGin_Param        	20000000	        68.1 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param5       	20000000	       122 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param20      	 5000000	       318 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParamWrite   	10000000	       162 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubStatic 	20000000	        95.1 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubParam  	10000000	       151 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubAll    	   50000	     30613 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusStatic  	20000000	        75.2 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusParam   	20000000	        99.3 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlus2Params 	20000000	       118 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusAll     	 1000000	      1318 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseStatic  	20000000	        75.7 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseParam   	20000000	        79.0 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Parse2Params 	20000000	        91.5 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseAll     	 1000000	      2476 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_StaticAll    	  100000	     20892 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/julienschmidt/go-http-routing-benchmark	31.804s

And the pull request benchmark data:

➜  go-http-routing-benchmark git:(master) go test -bench=Gin
#GithubAPI Routes: 203
   Gin: 51984 Bytes

#GPlusAPI Routes: 13
   Gin: 3936 Bytes

#ParseAPI Routes: 26
   Gin: 6912 Bytes

#Static Routes: 157
   Gin: 30480 Bytes

goos: darwin
goarch: amd64
pkg: github.com/julienschmidt/go-http-routing-benchmark
BenchmarkGin_Param        	10000000	       167 ns/op	      16 B/op	       1 allocs/op
BenchmarkGin_Param5       	 2000000	       769 ns/op	      96 B/op	       9 allocs/op
BenchmarkGin_Param20      	  300000	      3745 ns/op	    1472 B/op	      39 allocs/op
BenchmarkGin_ParamWrite   	 5000000	       261 ns/op	      16 B/op	       1 allocs/op
BenchmarkGin_GithubStatic 	 5000000	       323 ns/op	      32 B/op	       3 allocs/op
BenchmarkGin_GithubParam  	 2000000	       986 ns/op	     208 B/op	      10 allocs/op
BenchmarkGin_GithubAll    	   10000	    168442 ns/op	   28225 B/op	    1313 allocs/op
BenchmarkGin_GPlusStatic  	10000000	       156 ns/op	       8 B/op	       1 allocs/op
BenchmarkGin_GPlusParam   	 5000000	       308 ns/op	      32 B/op	       3 allocs/op
BenchmarkGin_GPlus2Params 	 2000000	       708 ns/op	     128 B/op	       6 allocs/op
BenchmarkGin_GPlusAll     	  300000	      7421 ns/op	    1048 B/op	      48 allocs/op
BenchmarkGin_ParseStatic  	10000000	       159 ns/op	       8 B/op	       1 allocs/op
BenchmarkGin_ParseParam   	 5000000	       351 ns/op	      48 B/op	       2 allocs/op
BenchmarkGin_Parse2Params 	 3000000	       572 ns/op	     112 B/op	       4 allocs/op
BenchmarkGin_ParseAll     	  200000	      9053 ns/op	    1168 B/op	      55 allocs/op
BenchmarkGin_StaticAll    	   20000	     79809 ns/op	   10416 B/op	     704 allocs/op
PASS
ok  	github.com/julienschmidt/go-http-routing-benchmark	32.995s

cc @appleboy

isgj · 2018-12-06T09:45:20Z

@thinkerou probably the reason of allocating those bytes is #1390 (comment)

thinkerou · 2018-12-06T10:05:33Z

@isgj but #1447 also add time, double! see #1447 (comment)

isgj · 2018-12-06T14:23:53Z

@thinkerou that's because of the defer here.
if you apply this patch

diff --git a/tree.go b/tree.go
index a0e1406..cb8814d 100644
--- a/tree.go
+++ b/tree.go
@@ -376,9 +376,6 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle
 // given path.
 func (n *node) getValue(path string, po Params, unescape bool) (handlers HandlersChain, p Params, relativePath string, tsr bool) {
 	p = po
-	defer func() {
-		relativePath = n.fullPath
-	}()
 walk: // Outer loop for walking the tree
 	for {
 		if len(path) > len(n.path) {
@@ -400,6 +397,7 @@ walk: // Outer loop for walking the tree
 					// We can recommend to redirect to the same URL without a
 					// trailing slash if a leaf exists for that path.
 					tsr = path == "/" && n.handlers != nil
+					relativePath = n.fullPath
 					return
 				}
 
@@ -440,10 +438,12 @@ walk: // Outer loop for walking the tree
 
 						// ... but we can't
 						tsr = len(path) == end+1
+						relativePath = n.fullPath
 						return
 					}
 
 					if handlers = n.handlers; handlers != nil {
+						relativePath = n.fullPath
 						return
 					}
 					if len(n.children) == 1 {
@@ -452,7 +452,7 @@ walk: // Outer loop for walking the tree
 						n = n.children[0]
 						tsr = n.path == "/" && n.handlers != nil
 					}
-
+					relativePath = n.fullPath
 					return
 
 				case catchAll:
@@ -473,6 +473,7 @@ walk: // Outer loop for walking the tree
 					}
 
 					handlers = n.handlers
+					relativePath = n.fullPath
 					return
 
 				default:
@@ -483,11 +484,13 @@ walk: // Outer loop for walking the tree
 			// We should have reached the node containing the handle.
 			// Check if this node has a handle registered.
 			if handlers = n.handlers; handlers != nil {
+				relativePath = n.fullPath
 				return
 			}
 
 			if path == "/" && n.wildChild && n.nType != root {
 				tsr = true
+				relativePath = n.fullPath
 				return
 			}
 
@@ -498,10 +501,11 @@ walk: // Outer loop for walking the tree
 					n = n.children[i]
 					tsr = (len(n.path) == 1 && n.handlers != nil) ||
 						(n.nType == catchAll && n.children[0].handlers != nil)
+					relativePath = n.fullPath
 					return
 				}
 			}
-
+			relativePath = n.fullPath
 			return
 		}
 
@@ -510,6 +514,7 @@ walk: // Outer loop for walking the tree
 		tsr = (path == "/") ||
 			(len(n.path) == len(path)+1 && n.path[len(path)] == '/' &&
 				path == n.path[:len(n.path)-1] && n.handlers != nil)
+		relativePath = n.fullPath
 		return
 	}
 }

the results are the same as master now.

tompave-roo · 2019-01-03T17:49:13Z

Hi, thank you for this! It would make things much easier on the instrumentation and reporting side.
Is there any update on whether this can be merged, and when?

tompave-roo · 2019-03-06T17:06:10Z

Bump.

tompave-roo · 2019-03-29T13:38:16Z

Does anyone have an interest in this? We can get it in shape to get it merged (remove the conflicts) if this is likely to be accepted.

denouche · 2019-04-08T07:33:41Z

What about this PR? it could be great for example for statistics usage, to be able to group requests instead of having response time by endpoint and path parameters.

gaffo · 2019-04-10T04:37:18Z

@denouche which PR?

@tompave-roo I have interest in it I just haven't had time to do anything. I went ahead and pulled the more ugly stack overflow example into my codebase and moved on.

denouche · 2019-04-10T11:09:53Z

This PR, the #1390 @gaffo
I mean, is it planned to merge it?
Thanks!

gaffo · 2019-04-10T14:49:20Z

The ticket says it needs to be up to date with master which is what the others are asking... If you have time and interest of suggest you do that.

…

On Wed, Apr 10, 2019, 04:10 Antoine Leveugle ***@***.***> wrote: This PR, the #390 <#390> @gaffo <https://github.com/gaffo> I mean, is it planned to merge it? Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1390 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAvLMY7qqQZjOKGUfro3HF1_qmT78ISks5vfcafgaJpZM4UjxfN> .

thinkerou · 2019-05-12T15:39:26Z

The pull request benchmark data:

➜  go-http-routing-benchmark git:(master) ✗ go test -bench=Gin
#GithubAPI Routes: 203
   Gin: 51984 Bytes

#GPlusAPI Routes: 13
   Gin: 3936 Bytes

#ParseAPI Routes: 26
   Gin: 6912 Bytes

#Static Routes: 157
   Gin: 30480 Bytes

goos: darwin
goarch: amd64
pkg: github.com/julienschmidt/go-http-routing-benchmark
BenchmarkGin_Param        	10000000	       206 ns/op	      16 B/op	       1 allocs/op
BenchmarkGin_Param5       	 2000000	       770 ns/op	      96 B/op	       9 allocs/op
BenchmarkGin_Param20      	  500000	      4327 ns/op	    1472 B/op	      39 allocs/op
BenchmarkGin_ParamWrite   	 5000000	       287 ns/op	      16 B/op	       1 allocs/op
BenchmarkGin_GithubStatic 	 5000000	       342 ns/op	      32 B/op	       3 allocs/op
BenchmarkGin_GithubParam  	 1000000	      1108 ns/op	     208 B/op	      10 allocs/op
BenchmarkGin_GithubAll    	   10000	    186752 ns/op	   28225 B/op	    1313 allocs/op
BenchmarkGin_GPlusStatic  	10000000	       178 ns/op	       8 B/op	       1 allocs/op
BenchmarkGin_GPlusParam   	 5000000	       379 ns/op	      32 B/op	       3 allocs/op
BenchmarkGin_GPlus2Params 	 2000000	       822 ns/op	     128 B/op	       6 allocs/op
BenchmarkGin_GPlusAll     	  200000	      7307 ns/op	    1048 B/op	      48 allocs/op
BenchmarkGin_ParseStatic  	10000000	       174 ns/op	       8 B/op	       1 allocs/op
BenchmarkGin_ParseParam   	 5000000	       316 ns/op	      48 B/op	       2 allocs/op
BenchmarkGin_Parse2Params 	 2000000	       654 ns/op	     112 B/op	       4 allocs/op
BenchmarkGin_ParseAll     	  200000	      9044 ns/op	    1168 B/op	      55 allocs/op
BenchmarkGin_StaticAll    	   20000	     78175 ns/op	   10416 B/op	     704 allocs/op
PASS
ok  	github.com/julienschmidt/go-http-routing-benchmark	32.811s

thinkerou · 2019-05-22T06:23:41Z

About the pull request which mentions proposal please see #1826 (comment), if you have any suggestion please ping me, thanks!

get matched route in context

4a38028

drsect0r mentioned this pull request Sep 12, 2018

add RelativePath #1447

Closed

fix conflicts

d0513be

Merge branch 'master' into master

c236352

thinkerou requested review from thinkerou, javierprovecho and appleboy October 23, 2018 14:15

isgj reviewed Oct 23, 2018

View reviewed changes

tree_test.go Show resolved Hide resolved

lftk and others added 5 commits October 23, 2018 23:02

Merge branch 'master' into master

9a495ae

route mismatch

57460a9

Add test to check if the route is in the context

37c19a1

Merge pull request #1 from isgj/route-context

1fae4c4

Add test to check if the route is in the context

Update README.md

7a8ad52

lftk force-pushed the master branch from a0256ea to 7a8ad52 Compare October 25, 2018 03:43

appleboy added the feature label Nov 8, 2018

appleboy added this to the 1.4 milestone Nov 8, 2018

appleboy previously approved these changes Nov 8, 2018

View reviewed changes

thinkerou approved these changes Dec 5, 2018

View reviewed changes

Merge branch 'master' into master

af609bf

thinkerou requested changes Dec 6, 2018

View reviewed changes

thinkerou added the break-backward label Dec 6, 2018

thinkerou mentioned this pull request Feb 15, 2019

context add RelativePath #1773

Closed

thinkerou modified the milestones: 1.4, 1.5 Mar 1, 2019

merge upstream

303675f

lftk dismissed appleboy’s stale review via 303675f March 9, 2019 07:05

thinkerou mentioned this pull request Mar 26, 2019

Hold matched route full path in the Context #1826

Merged

Merge branch 'master' into master

647f984

Merge branch 'master' into master

7817c01

thinkerou closed this May 26, 2019

drsect0r mentioned this pull request May 28, 2019

Wrong matched route path in the context #1917

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

get matched route from context #1390

get matched route from context #1390

lftk commented Jun 12, 2018

codecov bot commented Jun 12, 2018 •

edited

Loading

jlordiales commented Jul 12, 2018

nathancastelein commented Aug 24, 2018

drsect0r commented Sep 12, 2018 •

edited

Loading

drsect0r commented Sep 15, 2018 •

edited

Loading

thinkerou commented Oct 23, 2018 •

edited

Loading

thinkerou commented Oct 23, 2018

isgj commented Oct 23, 2018

drsect0r commented Nov 1, 2018

chenshuijin commented Nov 6, 2018

appleboy left a comment

Jim-Lambert-Bose commented Dec 5, 2018

thinkerou left a comment

thinkerou commented Dec 6, 2018

isgj commented Dec 6, 2018

thinkerou commented Dec 6, 2018 •

edited

Loading

isgj commented Dec 6, 2018

tompave-roo commented Jan 3, 2019

tompave-roo commented Mar 6, 2019

tompave-roo commented Mar 29, 2019

denouche commented Apr 8, 2019

gaffo commented Apr 10, 2019

denouche commented Apr 10, 2019 •

edited

Loading

gaffo commented Apr 10, 2019 via email

thinkerou commented May 12, 2019

thinkerou commented May 22, 2019

get matched route from context #1390

get matched route from context #1390

Conversation

lftk commented Jun 12, 2018

codecov bot commented Jun 12, 2018 • edited Loading

Codecov Report

jlordiales commented Jul 12, 2018

nathancastelein commented Aug 24, 2018

drsect0r commented Sep 12, 2018 • edited Loading

drsect0r commented Sep 15, 2018 • edited Loading

thinkerou commented Oct 23, 2018 • edited Loading

thinkerou commented Oct 23, 2018

isgj commented Oct 23, 2018

drsect0r commented Nov 1, 2018

chenshuijin commented Nov 6, 2018

appleboy left a comment

Choose a reason for hiding this comment

Jim-Lambert-Bose commented Dec 5, 2018

thinkerou left a comment

Choose a reason for hiding this comment

thinkerou commented Dec 6, 2018

isgj commented Dec 6, 2018

thinkerou commented Dec 6, 2018 • edited Loading

isgj commented Dec 6, 2018

tompave-roo commented Jan 3, 2019

tompave-roo commented Mar 6, 2019

tompave-roo commented Mar 29, 2019

denouche commented Apr 8, 2019

gaffo commented Apr 10, 2019

denouche commented Apr 10, 2019 • edited Loading

gaffo commented Apr 10, 2019 via email

thinkerou commented May 12, 2019

thinkerou commented May 22, 2019

codecov bot commented Jun 12, 2018 •

edited

Loading

drsect0r commented Sep 12, 2018 •

edited

Loading

drsect0r commented Sep 15, 2018 •

edited

Loading

thinkerou commented Oct 23, 2018 •

edited

Loading

thinkerou commented Dec 6, 2018 •

edited

Loading

denouche commented Apr 10, 2019 •

edited

Loading