obsservice: add logs_service proto for OTLP collector#93733
obsservice: add logs_service proto for OTLP collector#93733craig[bot] merged 2 commits intocockroachdb:masterfrom
Conversation
|
This change is |
5b9326e to
4a92b37
Compare
dhartunian
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @andreimatei)
andreimatei
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/obsservice/obspb/update-opentelemetry-proto.sh line 83 at r1 (raw file):
# into: # option go_package = "v1"; find $DEST_DIR -type f -name "*.proto" -exec sed -i '' -e "s/option go_package = \"go.opentelemetry.io\/proto\/otlp\/.*\/v1\"/option go_package = \"v1\"/" {} + ;
why did this change?
I don't see -i taking an argument in the man page.
aadityasondhi
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/obsservice/obspb/update-opentelemetry-proto.sh line 83 at r1 (raw file):
It errored on my local machine with:
./update-opentelemetry-proto.sh
Cloning opentelemetry-proto in /var/folders/sy/cstk63m54r992k2zybwh1_p80000gq/T/tmp.UX5yJRME.
Making a backup copy of opentelemetry-proto in /var/folders/sy/cstk63m54r992k2zybwh1_p80000gq/T/tmp.UVc0KC7t.
Copying protos.
Editing protos.
sed: 1: "opentelemetry-proto/com ...": invalid command code o
Deleted temp working directory /var/folders/sy/cstk63m54r992k2zybwh1_p80000gq/T/tmp.UX5yJRME
Did some digging and found: https://stackoverflow.com/questions/19456518/error-when-using-sed-with-find-command-on-os-x-invalid-command-code
"On the OSX version of sed, the -i option expects an extension argument so your command is actually parsed as the extension argument and the file path is interpreted as the command code.
Try adding the -e argument explicitly and giving '' as argument to -i"
I am assuming your machine is running Linux?
aadityasondhi
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/obsservice/obspb/update-opentelemetry-proto.sh line 83 at r1 (raw file):
Previously, aadityasondhi (Aaditya Sondhi) wrote…
It errored on my local machine with:
./update-opentelemetry-proto.sh Cloning opentelemetry-proto in /var/folders/sy/cstk63m54r992k2zybwh1_p80000gq/T/tmp.UX5yJRME. Making a backup copy of opentelemetry-proto in /var/folders/sy/cstk63m54r992k2zybwh1_p80000gq/T/tmp.UVc0KC7t. Copying protos. Editing protos. sed: 1: "opentelemetry-proto/com ...": invalid command code o Deleted temp working directory /var/folders/sy/cstk63m54r992k2zybwh1_p80000gq/T/tmp.UX5yJRMEDid some digging and found: https://stackoverflow.com/questions/19456518/error-when-using-sed-with-find-command-on-os-x-invalid-command-code
"On the OSX version of sed, the -i option expects an extension argument so your command is actually parsed as the extension argument and the file path is interpreted as the command code.
Try adding the -e argument explicitly and giving '' as argument to -i"I am assuming your machine is running Linux?
I wonder if this breaks the Linux version since -i doesn't take an argument. I can add a condition based on the OS in the script to make it work for both.
4a92b37 to
b361cf0
Compare
aadityasondhi
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/obsservice/obspb/update-opentelemetry-proto.sh line 83 at r1 (raw file):
Previously, aadityasondhi (Aaditya Sondhi) wrote…
I wonder if this breaks the Linux version since
-idoesn't take an argument. I can add a condition based on the OS in the script to make it work for both.
@andreimatei Fixed it to make it work with both OS. Let me know what you think.
andreimatei
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/obsservice/obspb/update-opentelemetry-proto.sh line 83 at r1 (raw file):
Previously, aadityasondhi (Aaditya Sondhi) wrote…
@andreimatei Fixed it to make it work with both OS. Let me know what you think.
damn BSDs
Let's find something that works across both of them, otherwise there's too many lines
I think this should work:
sed -i.bak -e ...
rm *.bak
And please extract this in a separate commit.
This patch adds the required protos for implementing the logs_service for the OTLP collector. This will allow cockroach to export events to an OTEL collector in the form of OTEL-compatible logs using the `LogsServiceClient`. Additionally, obsservice can ingest these logs as a `LogsServiceServer` when used in place of an OTEL collector. Epic: CRDB-16791 Release note: None
The OSX implementation of sed did not support the -i flag without parameters. This change makes the script compatible with both the linux and OSX implementation of sed. Release note: None
b361cf0 to
c16b213
Compare
aadityasondhi
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/obsservice/obspb/update-opentelemetry-proto.sh line 83 at r1 (raw file):
Previously, andreimatei (Andrei Matei) wrote…
damn BSDs
Let's find something that works across both of them, otherwise there's too many lines
I think this should work:sed -i.bak -e ... rm *.bakAnd please extract this in a separate commit.
Done.
|
bors r+ |
|
Build succeeded: |
This patch adds the required protos for implementing the logs_service for the OTLP collector. This will allow cockroach to export events to an OTEL collector in the form of OTEL-compatible logs using the
LogsServiceClient. Additionally, obsservice can ingest these logs as aLogsServiceServerwhen used in place of an OTEL collector.Epic: CRDB-16791
Release note: None