Redefine model/ and api_v2/ types as aliases to jaeger-idl/ types#6602
Redefine model/ and api_v2/ types as aliases to jaeger-idl/ types#6602yurishkuro merged 11 commits intojaegertracing:mainfrom
Conversation
Signed-off-by: nabil salah <nabil.salah203@gmail.com>
|
@yurishkuro
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6602 +/- ##
==========================================
- Coverage 96.21% 96.08% -0.14%
==========================================
Files 377 366 -11
Lines 21421 20949 -472
==========================================
- Hits 20611 20128 -483
- Misses 616 624 +8
- Partials 194 197 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
yurishkuro
left a comment
yurishkuro
left a comment
There was a problem hiding this comment.
Looks very promising! The link check is expected to fail, and unit test needs a bit of investigation, but doesn't look too serious. And the best part the e2e CI tests are green. Feels like a much better approach than the copying we tried before :-)
yes and it will ease the transition if at any moment you wanted to remove model files |
Signed-off-by: nabil salah <nabil.salah203@gmail.com>
|
when running unit tests locally I am seeing many logs like this a bit strange. |
Signed-off-by: nabil salah <nabil.salah203@gmail.com>
Signed-off-by: nabil salah <nabil.salah203@gmail.com>
i think this happened after I changed should I remove them or removing their proto registers init functions? |
|
I tracked one issue, in |
Signed-off-by: nabil salah <nabil.salah203@gmail.com>
|
|
|
grpc tests failing with Not sure why, you may need to make changes to plugin/storage/grpc/proto/storage.proto and regenerate - (gogoproto.customtype) = "github.com/jaegertracing/jaeger/model.TraceID",
+ (gogoproto.customtype) = "github.com/jaegertracing/jaeger-idl/model/v1.TraceID", |
Signed-off-by: nabil salah <nabil.salah203@gmail.com>
|
|
|
Run git submodule init, git submodule update --recursive |
Signed-off-by: nabil salah <nabil.salah203@gmail.com>
|
it runs with no problem but still giving e2e grpc fails |
Signed-off-by: nabil salah <nabil.salah203@gmail.com>
|
ok, we will need to troubleshoot, I don't have an obvious next step |
Signed-off-by: nabil salah <nabil.salah203@gmail.com>
|
try this patch. I don't know why gogo still encounters the old model, but I was able to reproduce the error in a unit test and this change below fixed it. --- a/pkg/gogocodec/codec.go
+++ b/pkg/gogocodec/codec.go
@@ -17,6 +17,9 @@ import (
const (
jaegerProtoGenPkgPath = "github.com/jaegertracing/jaeger-idl/proto-gen"
jaegerModelPkgPath = "github.com/jaegertracing/jaeger-idl/model/v1"
+
+ jaegerProtoGenPkgPathOld = "github.com/jaegertracing/jaeger/proto-gen"
+ jaegerModelPkgPathOld = "github.com/jaegertracing/jaeger/model"
)
var defaultCodec encoding.CodecV2
@@ -89,5 +92,11 @@ func useGogo(t reflect.Type) bool {
if strings.HasPrefix(pkg, jaegerModelPkgPath) {
return true
}
+ if strings.HasPrefix(pkg, jaegerProtoGenPkgPathOld) {
+ return true
+ }
+ if strings.HasPrefix(pkg, jaegerModelPkgPathOld) {
+ return true
+ }
return false
} |
Signed-off-by: nabil salah <nabil.salah203@gmail.com>
yurishkuro
changed the title
this is weird because we changed proto files that use model.proto already don't know why it happened |
|
Looks like we made it. This is awesome! I'm going to ignore codecov gap, we will soon be able to clean up that code anyway. |
2 participants
Which problem is this PR solving?
Description of the changes
modelsandapi_v2modelsHow was this change tested?
make lint,make testChecklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test