-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add ability to generate non nullable messages, convert Resource to test the change #2005
Add ability to generate non nullable messages, convert Resource to test the change #2005
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2005 +/- ##
==========================================
- Coverage 92.10% 92.08% -0.02%
==========================================
Files 278 278
Lines 17030 16978 -52
==========================================
- Hits 15685 15635 -50
+ Misses 924 923 -1
+ Partials 421 420 -1
Continue to review full report at Codecov.
|
bff7ef3
to
8e4adc1
Compare
d303b0b
to
518614a
Compare
cmd/pdatagen/internal/base_fields.go
Outdated
// If no ${lowerFieldName} available, it creates an empty message and associates it with this ${structName}. | ||
// | ||
// Empty initialized ${structName} will return "nil" ${returnType}. | ||
// | ||
// Important: This causes a runtime error if IsNil() returns "true". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comments no longer apply, since it is not a pointer anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It still applies because this is the getter for a non double pointer message, but the parent struct that embeds this message can still be double pointer.
77a995e
to
d62d1c2
Compare
@tigrannajaryan PTAL |
7388938
to
ebad03b
Compare
Conceptually LGTM. Should we start with slices first though since that will give us the most gains? |
@tigrannajaryan can you clarify "Conceptually LGTM. Should we start with slices first though since that will give us the most gains?". Instead of changing Resource to also add support to have slices of "messageValue" and start changing some of the slices? I think it is fine, but "Resource" also gives us something and it is still important to have one object that is not part of the slice I think. |
If I understand correctly one of the goals of this change is improving performance. We will not see much gains from changing the Resource from pointer to value type. The most gains can be seen from the most numerous structs, e.g. Span, which are part of InstrumentationSpans slice. I am fine if you want to still go ahead with the Resource and use it as the playground for the upcoming changes for other structs. |
That was my intention, to use this as a playground. Changing a slice as well would make the change to big. Also in this PR we improved couple of other things like the service name. |
@bogdandrutu please resolve the conflicts. |
…st the change Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
a653909
to
66e95a7
Compare
…ry#2005) * Split stdout exporter into stdouttrace and stdoutmetric Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com> * Remove unused options from stdouttrace and stdoutmetric exporters Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com> * Update stdout exporter references in website docs Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com> * Update docs to include correct import paths, properly describe exporter scope Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com> * Remove pointless options to disable signals from what are now single-signal exporters Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>
…_client`, and make it contains all symbols needed. (open-telemetry#2005) Fix open-telemetry#1998 Delegate all API calls of gRPC into `opentelemetry_exporter_otlp_grpc_client`, and make it contains all symbols needed.
Signed-off-by: Bogdan Drutu bogdandrutu@gmail.com