Skip to content
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

Resource sdk Implementation #502

Merged
merged 37 commits into from
Jan 22, 2021
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
a27a5e4
resource api
lalitb Jan 5, 2021
abe8287
format and add tests
lalitb Jan 5, 2021
083b169
fix bazel
lalitb Jan 5, 2021
b61acdd
fix bazel another try
lalitb Jan 5, 2021
4540bc5
more fixes
lalitb Jan 5, 2021
dfe857c
fix resouce interface constructor
lalitb Jan 5, 2021
0ae679e
Merge branch 'master' into resources
lalitb Jan 5, 2021
7673007
fix review comments
lalitb Jan 9, 2021
d5b389f
revert change
lalitb Jan 11, 2021
9fbe8be
revert change
lalitb Jan 11, 2021
208f231
revert change
lalitb Jan 11, 2021
60034d9
fix merge conflict
lalitb Jan 12, 2021
04e9de8
Merge branch 'master' into resources
lalitb Jan 12, 2021
bfdd7e1
fix attributes_utils.h
lalitb Jan 12, 2021
2abd049
fix review comments
lalitb Jan 12, 2021
f71a47c
fix attributes
lalitb Jan 12, 2021
f7e4909
add doc
lalitb Jan 12, 2021
bc3bebc
fix static
lalitb Jan 12, 2021
48a30d4
fix test
lalitb Jan 12, 2021
d8d015a
review comments
lalitb Jan 17, 2021
48ab9ea
fix bazel build
lalitb Jan 17, 2021
f515165
remove un-necesaary include
lalitb Jan 17, 2021
5b01258
add test for different resoure types
lalitb Jan 18, 2021
e9745f1
fix otlp
lalitb Jan 18, 2021
84d5405
Merge branch 'master' into resources
lalitb Jan 18, 2021
01fd510
fix valgrind error
lalitb Jan 18, 2021
c60fefc
wMerge branch 'resources' of github.com:lalitb/opentelemetry-cpp into…
lalitb Jan 18, 2021
b023c2a
modify simple trace example to use resource
lalitb Jan 18, 2021
1586018
fix simple example
lalitb Jan 18, 2021
9158d71
make resource ctor private
lalitb Jan 21, 2021
db2412b
Merge branch 'master' into resources
lalitb Jan 21, 2021
35dbf99
fix access
lalitb Jan 21, 2021
5e67443
Merge branch 'resources' of github.com:lalitb/opentelemetry-cpp into …
lalitb Jan 21, 2021
adbd86d
fixes
lalitb Jan 21, 2021
ef45994
fix comment
lalitb Jan 21, 2021
a272b92
Update sdk/src/resource/resource.cc
lalitb Jan 22, 2021
8cd247e
Merge branch 'master' into resources
lalitb Jan 22, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/batch/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ cc_binary(
deps = [
"//api",
"//exporters/ostream:ostream_span_exporter",
"//sdk/src/resource",
"//sdk/src/trace",
],
)
3 changes: 2 additions & 1 deletion examples/batch/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ add_executable(batch_span_processor_example main.cc)

target_link_libraries(
batch_span_processor_example ${CMAKE_THREAD_LIBS_INIT} ${CORE_RUNTIME_LIBS}
opentelemetry_exporter_ostream_span opentelemetry_trace)
opentelemetry_exporter_ostream_span opentelemetry_trace
opentelemetry_resources)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we link this into opentelemetry_trace, like we do opentelemetry_common? Then we wouldn't need to specify it alongside opentelemetry_trace everywhere. opentelemetry_trace has a hard dependency on opentelemetry_resource anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, done.

1 change: 1 addition & 0 deletions examples/multithreaded/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ cc_binary(
deps = [
"//api",
"//exporters/ostream:ostream_span_exporter",
"//sdk/src/resource",
"//sdk/src/trace",
],
)
5 changes: 3 additions & 2 deletions examples/multithreaded/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
include_directories(${CMAKE_SOURCE_DIR}/exporters/ostream/include)

add_executable(example_multithreaded main.cc)
target_link_libraries(example_multithreaded ${CMAKE_THREAD_LIBS_INIT}
opentelemetry_trace opentelemetry_exporter_ostream_span)
target_link_libraries(
example_multithreaded ${CMAKE_THREAD_LIBS_INIT} opentelemetry_trace
opentelemetry_resources opentelemetry_exporter_ostream_span)
1 change: 1 addition & 0 deletions examples/otlp/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ cc_binary(
":foo_library",
"//api",
"//exporters/otlp:otlp_exporter",
"//sdk/src/resource",
"//sdk/src/trace",
],
)
1 change: 1 addition & 0 deletions examples/otlp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ target_link_libraries(
${CMAKE_THREAD_LIBS_INIT}
otlp_foo_library
opentelemetry_trace
opentelemetry_resources
${CORE_RUNTIME_LIBS}
opentelemetry_exporter_otprotocol
gRPC::grpc++)
1 change: 1 addition & 0 deletions examples/simple/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ cc_binary(
":foo_library",
"//api",
"//exporters/ostream:ostream_span_exporter",
"//sdk/src/resource",
"//sdk/src/trace",
],
)
2 changes: 1 addition & 1 deletion examples/simple/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ target_link_libraries(foo_library opentelemetry_exporter_ostream_span

add_executable(example_simple main.cc)
target_link_libraries(example_simple ${CMAKE_THREAD_LIBS_INIT} foo_library
opentelemetry_trace)
opentelemetry_trace opentelemetry_resources)
6 changes: 4 additions & 2 deletions examples/simple/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ void initTracer()
new opentelemetry::exporter::trace::OStreamSpanExporter);
auto processor = std::shared_ptr<sdktrace::SpanProcessor>(
new sdktrace::SimpleSpanProcessor(std::move(exporter)));
auto provider = nostd::shared_ptr<opentelemetry::trace::TracerProvider>(
new sdktrace::TracerProvider(processor));
auto provider =
nostd::shared_ptr<opentelemetry::trace::TracerProvider>(new sdktrace::TracerProvider(
processor, std::make_shared<opentelemetry::sdk::trace::AlwaysOnSampler>(),
opentelemetry::sdk::resource::Resource::Create({})));
// Set the global trace provider
opentelemetry::trace::Provider::SetTracerProvider(provider);
}
Expand Down
1 change: 1 addition & 0 deletions examples/zpages/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ cc_binary(
deps = [
"//ext:headers",
"//ext/src/zpages",
"//sdk/src/resource",
"//sdk/src/trace",
],
)
1 change: 1 addition & 0 deletions exporters/otlp/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ cc_test(
deps = [
":otlp_exporter",
"//api",
"//sdk/src/resource",
"@com_google_googletest//:gtest_main",
],
)
Expand Down
2 changes: 1 addition & 1 deletion ext/src/zpages/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ set_target_properties(opentelemetry_zpages PROPERTIES EXPORT_NAME zpages)

target_link_libraries(
opentelemetry_zpages PUBLIC opentelemetry_ext opentelemetry_api
opentelemetry_trace)
opentelemetry_trace opentelemetry_resources)

install(
TARGETS opentelemetry_zpages
Expand Down
2 changes: 2 additions & 0 deletions ext/test/zpages/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ cc_test(
],
deps = [
"//ext/src/zpages",
"//sdk/src/resource",
"//sdk/src/trace",
"@com_google_googletest//:gtest_main",
],
Expand All @@ -29,6 +30,7 @@ cc_test(
],
deps = [
"//ext/src/zpages",
"//sdk/src/resource",
"//sdk/src/trace",
"@com_google_googletest//:gtest_main",
],
Expand Down
75 changes: 75 additions & 0 deletions sdk/include/opentelemetry/sdk/resource/resource.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
#pragma once

#include "opentelemetry/sdk/common/attribute_utils.h"
#include "opentelemetry/sdk/resource/resource_detector.h"
#include "opentelemetry/sdk/version/version.h"
#include "opentelemetry/version.h"

#include <memory>
#include <sstream>
#include <unordered_map>

OPENTELEMETRY_BEGIN_NAMESPACE
namespace sdk
{
namespace resource
{

using ResourceAttributes =
std::unordered_map<std::string, opentelemetry::sdk::common::OwnedAttributeValue>;

class Resource
{
public:
const ResourceAttributes &GetAttributes() const noexcept;

/**
* Returns a new, merged {@link Resource} by merging the current Resource
* with the other Resource. In case of a collision, current Resource takes
* precedence.
*
* @param other the Resource that will be merged with this.
* @returns the newly merged Resource.
*/

Resource Merge(const Resource &other) noexcept;

/**
* Returns a newly created Resource with the specified attributes.
* It adds (merge) SDK attributes and OTEL attributes before returning.
* @param attributes for this resource
* @returns the newly created Resource.
*/

static Resource Create(const ResourceAttributes &attributes);

/**
* Returns an Empty resource.
*/

static Resource &GetEmpty();

/**
* Returns a Resource that indentifies the SDK in use.
*/

static Resource &GetDefault();

protected:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is protected necessary or just private is fine?

Copy link
Member Author

@lalitb lalitb Jan 21, 2021

Choose a reason for hiding this comment

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

protected helps creating unit test case - to subclass Resource and create instance of it for testing. We are doing this in resources_test.cc. I have modified the comment to indicate it's protected :)

/**
* The constructor is private and only for use internally by the class and
* inside ResourceDetector class.
* Users should use the Create factory method to obtain a Resource
* instance.
*/
Resource(const ResourceAttributes &attributes = ResourceAttributes()) noexcept;

private:
ResourceAttributes attributes_;

friend class OTELResourceDetector;
};

} // namespace resource
} // namespace sdk
OPENTELEMETRY_END_NAMESPACE
35 changes: 35 additions & 0 deletions sdk/include/opentelemetry/sdk/resource/resource_detector.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#pragma once

#include "opentelemetry/nostd/unique_ptr.h"
#include "opentelemetry/version.h"

OPENTELEMETRY_BEGIN_NAMESPACE
namespace sdk
{
namespace resource
{

class Resource;

/**
* Interface for a Resource Detector
*/
class ResourceDetector
{
public:
virtual Resource Detect() = 0;
};

/**
* OTelResourceDetector to detect the presence of and create a Resource
* from the OTEL_RESOURCE_ATTRIBUTES environment variable.
*/
class OTELResourceDetector : public ResourceDetector
{
public:
Resource Detect() noexcept override;
};

} // namespace resource
} // namespace sdk
OPENTELEMETRY_END_NAMESPACE
6 changes: 5 additions & 1 deletion sdk/include/opentelemetry/sdk/trace/tracer.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include "opentelemetry/sdk/common/atomic_shared_ptr.h"
#include "opentelemetry/sdk/resource/resource.h"
#include "opentelemetry/sdk/trace/processor.h"
#include "opentelemetry/sdk/trace/samplers/always_on.h"
#include "opentelemetry/trace/noop.h"
Expand All @@ -23,7 +24,9 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th
* nullptr.
*/
explicit Tracer(std::shared_ptr<SpanProcessor> processor,
std::shared_ptr<Sampler> sampler = std::make_shared<AlwaysOnSampler>()) noexcept;
std::shared_ptr<Sampler> sampler = std::make_shared<AlwaysOnSampler>(),
const opentelemetry::sdk::resource::Resource &resource =
opentelemetry::sdk::resource::Resource::Create({})) noexcept;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this one may leak a nullptr exception from the default value.

I.e. the default argument won't be retained. AFAIK it should be ok to not give this argument a default value and require the TracerProvider to pass one into the tracer.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, default argument for reference can leak. Have removed default now.


/**
* Set the span processor associated with this tracer.
Expand Down Expand Up @@ -57,6 +60,7 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th
private:
opentelemetry::sdk::AtomicSharedPtr<SpanProcessor> processor_;
const std::shared_ptr<Sampler> sampler_;
const opentelemetry::sdk::resource::Resource &resource_;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we shouldn't make this a reference, but hold the resource by value. Theoretically, we support using a Tracer without a TracerProvider (I remember that was a requirement that came up early in the design process). Having a copy of the resource in the Tracer makes this design safer and doesn't cost much.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed.

};
} // namespace trace
} // namespace sdk
Expand Down
17 changes: 13 additions & 4 deletions sdk/include/opentelemetry/sdk/trace/tracer_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <string>

#include "opentelemetry/nostd/shared_ptr.h"
#include "opentelemetry/sdk/resource/resource.h"
#include "opentelemetry/sdk/trace/processor.h"
#include "opentelemetry/sdk/trace/samplers/always_on.h"
#include "opentelemetry/sdk/trace/tracer.h"
Expand All @@ -25,9 +26,10 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider
* @param sampler The sampler for this tracer provider. This must
* not be a nullptr.
*/
explicit TracerProvider(
std::shared_ptr<SpanProcessor> processor,
std::shared_ptr<Sampler> sampler = std::make_shared<AlwaysOnSampler>()) noexcept;
explicit TracerProvider(std::shared_ptr<SpanProcessor> processor,
std::shared_ptr<Sampler> sampler = std::make_shared<AlwaysOnSampler>(),
opentelemetry::sdk::resource::Resource &&resource =
opentelemetry::sdk::resource::Resource::Create({})) noexcept;

opentelemetry::nostd::shared_ptr<opentelemetry::trace::Tracer> GetTracer(
nostd::string_view library_name,
Expand All @@ -48,10 +50,16 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider

/**
* Obtain the sampler associated with this tracer provider.
* @return The span processor for this tracer provider.
* @return The sampler for this tracer provider.
*/
std::shared_ptr<Sampler> GetSampler() const noexcept;

/**
* Obtain the resource associated with this tracer provider.
* @return The resource for this tracer provider.
*/
const opentelemetry::sdk::resource::Resource &GetResource() const noexcept;

/**
* Shutdown the span processor associated with this tracer provider.
*/
Expand All @@ -61,6 +69,7 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider
opentelemetry::sdk::AtomicSharedPtr<SpanProcessor> processor_;
std::shared_ptr<opentelemetry::trace::Tracer> tracer_;
const std::shared_ptr<Sampler> sampler_;
const opentelemetry::sdk::resource::Resource resource_;
};
} // namespace trace
} // namespace sdk
Expand Down
1 change: 1 addition & 0 deletions sdk/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ add_subdirectory(trace)
add_subdirectory(metrics)
add_subdirectory(logs)
add_subdirectory(version)
add_subdirectory(resource)
26 changes: 26 additions & 0 deletions sdk/src/resource/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Copyright 2020, OpenTelemetry Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

package(default_visibility = ["//visibility:public"])

cc_library(
name = "resource",
srcs = glob(["**/*.cc"]),
hdrs = glob(["**/*.h"]),
include_prefix = "src/resource",
deps = [
"//api",
"//sdk:headers",
],
)
16 changes: 16 additions & 0 deletions sdk/src/resource/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
add_library(opentelemetry_resources resource.cc resource_detector.cc)

set_target_properties(opentelemetry_resources PROPERTIES EXPORT_NAME resources)

target_link_libraries(opentelemetry_resources opentelemetry_common)

target_include_directories(
opentelemetry_resources
PUBLIC "$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/sdk/include>")

install(
TARGETS opentelemetry_resources
EXPORT "${PROJECT_NAME}-target"
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})
Loading