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

Move implementation from SDK header files to SDK *.cc #1429

Open
marcalff opened this issue Jun 3, 2022 · 3 comments
Open

Move implementation from SDK header files to SDK *.cc #1429

marcalff opened this issue Jun 3, 2022 · 3 comments
Assignees
Labels
bug Something isn't working do-not-stale priority:p2 Issues that are not blocking

Comments

@marcalff
Copy link
Member

marcalff commented Jun 3, 2022

This is a remaining item identified during review of PR #1420

For all the static variables found in SDK header files,
consider (to discuss) to either:

  • move the code to the *.cc files instead,
  • remove the static keyword,
  • add comments to explain why use of static is safe.

List of variables identified:

File opentelemetry/sdk/common/empty_attributes.h

  static const std::array<std::pair<std::string, int>, 0> array{};
  static const opentelemetry::common::KeyValueIterableView<
      std::array<std::pair<std::string, int>, 0>>
      kEmptyAttributes(array);

File opentelemetry/sdk/metrics/aggregation/drop_aggregation.h

    static DropPointData point_data;

File opentelemetry/sdk/metrics/exemplar/always_sample_filter.h

    static nostd::shared_ptr<ExemplarFilter> alwaysSampleFilter{new AlwaysSampleFilter{}};

File opentelemetry/sdk/metrics/view/view_registry.h

      static View view("otel-default-view");

File opentelemetry/sdk/resource/experimental_semantic_conventions.h

static const std::unordered_map<uint32_t, const char *> attribute_ids = { ... };

This one (attribute_ids) probably needs more investigation before doing any changes (strings resolved at compile time or runtime ?).

File opentelemetry/sdk/trace/multi_recordable.h

    static std::unique_ptr<Recordable> empty(nullptr);

File opentelemetry/sdk/trace/span_data.h

      static opentelemetry::sdk::resource::Resource resource =

      static std::unique_ptr<opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary>
          instrumentation_library =

Code cleanup, low priority.

@marcalff marcalff added the bug Something isn't working label Jun 3, 2022
@lalitb lalitb added good first issue Good for newcomers help wanted Good for taking. Extra help will be provided by maintainers labels Jun 3, 2022
@esigo esigo added the priority:p2 Issues that are not blocking label Jun 21, 2022
@lalitb lalitb removed help wanted Good for taking. Extra help will be provided by maintainers good first issue Good for newcomers labels Jul 18, 2022
@marcalff
Copy link
Member Author

As discussed in community meeting, expanding the scope from "moving static variables to *.cc" to "moving all code to *.cc".

@marcalff marcalff changed the title Move static variables from SDK header files to SDK *.cc Move implementation from SDK header files to SDK *.cc Jul 21, 2022
@github-actions
Copy link

This issue was marked as stale due to lack of activity.

@github-actions github-actions bot added the Stale label Sep 20, 2022
@lalitb lalitb removed the Stale label Sep 20, 2022
@github-actions
Copy link

This issue was marked as stale due to lack of activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working do-not-stale priority:p2 Issues that are not blocking
Projects
None yet
Development

No branches or pull requests

3 participants