Skip to content

Add Recordable implementation for otprotocol exporter#49

Merged
rnburn merged 49 commits into
open-telemetry:masterfrom
rnburn:otprotocol
Apr 6, 2020
Merged

Add Recordable implementation for otprotocol exporter#49
rnburn merged 49 commits into
open-telemetry:masterfrom
rnburn:otprotocol

Conversation

@rnburn
Copy link
Copy Markdown
Contributor

@rnburn rnburn commented Mar 12, 2020

No description provided.

Comment thread WORKSPACE Outdated
Comment thread bazel/opentelemetry_proto.BUILD Outdated
Comment thread bazel/opentelemetry_proto.BUILD
Comment thread ci/do_ci.sh
Comment thread sdk/proto/opentelemetry/README
@@ -0,0 +1,28 @@
#include "src/exporter/otprotocol/recordable.h"

OPENTELEMETRY_BEGIN_NAMESPACE
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should exporters live in the internal namespace?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I separated out into an exporters folder, similar to how it's done for go and java.

Comment thread sdk/src/trace/recordable.h
Comment thread WORKSPACE Outdated
Comment thread ci/install_protobuf.sh
Comment thread .circleci/config.yml Outdated
- run: ./ci/install_bazelisk.sh
- run: ./ci/install_gcc48.sh
- run: CC=/usr/bin/g++-4.8 ./ci/do_ci.sh bazel.test
- run: CC=/usr/bin/g++-4.8 ./ci/do_ci.sh bazel.test_api
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API is gcc 4.8 compatible but SDK is not?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protobuf can't work with gcc-4.8.

Maybe I should exclude just some portions of the sdk from legacy compiler tests?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you. Sorry to hear about protobuf.

@@ -0,0 +1,33 @@
#pragma once

#include "src/trace/recordable.h"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think headers needed to implement exporters should be placed in sdk/include. However, this can be done in a different PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved Recodable into the headers folder.

@rnburn rnburn merged commit 4ef44ed into open-telemetry:master Apr 6, 2020
@nadiaciobanu nadiaciobanu mentioned this pull request Jul 3, 2020
GerHobbelt pushed a commit to GerHobbelt/opentelemetry-cpp that referenced this pull request Jul 27, 2024
 [SDK] Added reserve for spans array in BatchSpanProcessor. (open-telemetry#2724)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants