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.
|
bogdandrutu
force-pushed
the
addmessagevalue
branch
2 times, most recently
from
bff7ef3
to
8e4adc1
Compare
bogdandrutu
force-pushed
the
addmessagevalue
branch
3 times, most recently
from
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.
This comments no longer apply, since it is not a pointer anymore.
There was a problem hiding this comment.
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.
bogdandrutu
force-pushed
the
addmessagevalue
branch
2 times, most recently
from
77a995e
to
d62d1c2
Compare
|
@tigrannajaryan PTAL |
bogdandrutu
force-pushed
the
addmessagevalue
branch
from
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>
bogdandrutu
force-pushed
the
addmessagevalue
branch
from
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