Skip to content

Commit 8aa248f

Browse files
marcalffmarcalff
committed
Refresh code from PR #2518, to fix review comments.
1 parent 7aad664 commit 8aa248f

File tree

5 files changed

+121
-116
lines changed

5 files changed

+121
-116
lines changed

‎sdk/include/opentelemetry/sdk/configuration/document_node.h‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -32,25 +32,26 @@ class DocumentNode
3232

3333
virtual std::string Key() const = 0;
3434

35-
virtual bool AsBoolean() = 0;
36-
virtual std::size_t AsInteger() = 0;
37-
virtual double AsDouble() = 0;
38-
virtual std::string AsString() = 0;
35+
virtual bool AsBoolean() const = 0;
36+
virtual std::size_t AsInteger() const = 0;
37+
virtual double AsDouble() const = 0;
38+
virtual std::string AsString() const = 0;
3939

40-
virtual std::unique_ptr<DocumentNode> GetRequiredChildNode(const std::string &name) = 0;
41-
virtual std::unique_ptr<DocumentNode> GetChildNode(const std::string &name) = 0;
40+
virtual std::unique_ptr<DocumentNode> GetRequiredChildNode(const std::string &name) const = 0;
41+
virtual std::unique_ptr<DocumentNode> GetChildNode(const std::string &name) const = 0;
4242

43-
virtual bool GetRequiredBoolean(const std::string &name) = 0;
44-
virtual bool GetBoolean(const std::string &name, bool default_value) = 0;
43+
virtual bool GetRequiredBoolean(const std::string &name) const = 0;
44+
virtual bool GetBoolean(const std::string &name, bool default_value) const = 0;
4545

46-
virtual std::size_t GetRequiredInteger(const std::string &name) = 0;
47-
virtual std::size_t GetInteger(const std::string &name, std::size_t default_value) = 0;
46+
virtual std::size_t GetRequiredInteger(const std::string &name) const = 0;
47+
virtual std::size_t GetInteger(const std::string &name, std::size_t default_value) const = 0;
4848

49-
virtual double GetRequiredDouble(const std::string &name) = 0;
50-
virtual double GetDouble(const std::string &name, double default_value) = 0;
49+
virtual double GetRequiredDouble(const std::string &name) const = 0;
50+
virtual double GetDouble(const std::string &name, double default_value) const = 0;
5151

52-
virtual std::string GetRequiredString(const std::string &name) = 0;
53-
virtual std::string GetString(const std::string &name, const std::string &default_value) = 0;
52+
virtual std::string GetRequiredString(const std::string &name) const = 0;
53+
virtual std::string GetString(const std::string &name,
54+
const std::string &default_value) const = 0;
5455

5556
virtual DocumentNodeConstIterator begin() const = 0;
5657
virtual DocumentNodeConstIterator end() const = 0;
@@ -62,12 +63,12 @@ class DocumentNode
6263
virtual PropertiesNodeConstIterator end_properties() const = 0;
6364

6465
protected:
65-
std::string DoSubstitution(const std::string &text);
66-
std::string DoOneSubstitution(const std::string &text);
66+
std::string DoSubstitution(const std::string &text) const;
67+
std::string DoOneSubstitution(const std::string &text) const;
6768

68-
bool BooleanFromString(const std::string &value);
69-
std::size_t IntegerFromString(const std::string &value);
70-
double DoubleFromString(const std::string &value);
69+
bool BooleanFromString(const std::string &value) const;
70+
std::size_t IntegerFromString(const std::string &value) const;
71+
double DoubleFromString(const std::string &value) const;
7172
};
7273

7374
class DocumentNodeConstIteratorImpl

‎sdk/include/opentelemetry/sdk/configuration/ryml_document_node.h‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,25 +29,25 @@ class RymlDocumentNode : public DocumentNode
2929

3030
std::string Key() const override;
3131

32-
bool AsBoolean() override;
33-
std::size_t AsInteger() override;
34-
double AsDouble() override;
35-
std::string AsString() override;
32+
bool AsBoolean() const override;
33+
std::size_t AsInteger() const override;
34+
double AsDouble() const override;
35+
std::string AsString() const override;
3636

37-
std::unique_ptr<DocumentNode> GetRequiredChildNode(const std::string &name) override;
38-
std::unique_ptr<DocumentNode> GetChildNode(const std::string &name) override;
37+
std::unique_ptr<DocumentNode> GetRequiredChildNode(const std::string &name) const override;
38+
std::unique_ptr<DocumentNode> GetChildNode(const std::string &name) const override;
3939

40-
bool GetRequiredBoolean(const std::string &name) override;
41-
bool GetBoolean(const std::string &name, bool default_value) override;
40+
bool GetRequiredBoolean(const std::string &name) const override;
41+
bool GetBoolean(const std::string &name, bool default_value) const override;
4242

43-
std::size_t GetRequiredInteger(const std::string &name) override;
44-
std::size_t GetInteger(const std::string &name, std::size_t default_value) override;
43+
std::size_t GetRequiredInteger(const std::string &name) const override;
44+
std::size_t GetInteger(const std::string &name, std::size_t default_value) const override;
4545

46-
double GetRequiredDouble(const std::string &name) override;
47-
double GetDouble(const std::string &name, double default_value) override;
46+
double GetRequiredDouble(const std::string &name) const override;
47+
double GetDouble(const std::string &name, double default_value) const override;
4848

49-
std::string GetRequiredString(const std::string &name) override;
50-
std::string GetString(const std::string &name, const std::string &default_value) override;
49+
std::string GetRequiredString(const std::string &name) const override;
50+
std::string GetString(const std::string &name, const std::string &default_value) const override;
5151

5252
DocumentNodeConstIterator begin() const override;
5353
DocumentNodeConstIterator end() const override;
@@ -59,8 +59,8 @@ class RymlDocumentNode : public DocumentNode
5959
PropertiesNodeConstIterator end_properties() const override;
6060

6161
private:
62-
ryml::ConstNodeRef GetRequiredRymlChildNode(const std::string &name);
63-
ryml::ConstNodeRef GetRymlChildNode(const std::string &name);
62+
ryml::ConstNodeRef GetRequiredRymlChildNode(const std::string &name) const;
63+
ryml::ConstNodeRef GetRymlChildNode(const std::string &name) const;
6464

6565
ryml::ConstNodeRef node_;
6666
std::size_t depth_;

‎sdk/src/configuration/document_node.cc‎

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,9 @@
33

44
#include <cctype>
55
#include <cstdlib>
6-
#include <ostream>
76
#include <string>
87

98
#include "opentelemetry/sdk/common/env_variables.h"
10-
#include "opentelemetry/sdk/common/global_log_handler.h"
119
#include "opentelemetry/sdk/configuration/document_node.h"
1210
#include "opentelemetry/sdk/configuration/invalid_schema_exception.h"
1311
#include "opentelemetry/version.h"
@@ -26,7 +24,7 @@ namespace configuration
2624
* - Some text with ${SUBSTITUTION} in it
2725
* - Multiple ${SUBSTITUTION_A} substitutions ${SUBSTITUTION_B} in the line
2826
*/
29-
std::string DocumentNode::DoSubstitution(const std::string &text)
27+
std::string DocumentNode::DoSubstitution(const std::string &text) const
3028
{
3129
static std::string begin_token{"${"};
3230
static std::string end_token{"}"};
@@ -90,9 +88,9 @@ std::string DocumentNode::DoSubstitution(const std::string &text)
9088
- ${ENV_NAME:-fallback} (including when ENV_NAME is actually "env")
9189
- ${env:ENV_NAME:-fallback}
9290
*/
93-
std::string DocumentNode::DoOneSubstitution(const std::string &text)
91+
std::string DocumentNode::DoOneSubstitution(const std::string &text) const
9492
{
95-
static std::string illegal_msg{"Illegal substitution expression"};
93+
static std::string illegal_msg{"Illegal substitution expression: "};
9694

9795
static std::string env_token{"env:"};
9896
static std::string env_with_replacement{"env:-"};
@@ -112,29 +110,33 @@ std::string DocumentNode::DoOneSubstitution(const std::string &text)
112110

113111
if (len < 4)
114112
{
115-
OTEL_INTERNAL_LOG_ERROR(illegal_msg << ": " << text);
116-
throw InvalidSchemaException(illegal_msg);
113+
std::string message = illegal_msg;
114+
message.append(text);
115+
throw InvalidSchemaException(message);
117116
}
118117

119118
c = text[0];
120119
if (c != '$')
121120
{
122-
OTEL_INTERNAL_LOG_ERROR(illegal_msg << ": " << text);
123-
throw InvalidSchemaException(illegal_msg);
121+
std::string message = illegal_msg;
122+
message.append(text);
123+
throw InvalidSchemaException(message);
124124
}
125125

126126
c = text[1];
127127
if (c != '{')
128128
{
129-
OTEL_INTERNAL_LOG_ERROR(illegal_msg << ": " << text);
130-
throw InvalidSchemaException(illegal_msg);
129+
std::string message = illegal_msg;
130+
message.append(text);
131+
throw InvalidSchemaException(message);
131132
}
132133

133134
c = text[len - 1];
134135
if (c != '}')
135136
{
136-
OTEL_INTERNAL_LOG_ERROR(illegal_msg << ": " << text);
137-
throw InvalidSchemaException(illegal_msg);
137+
std::string message = illegal_msg;
138+
message.append(text);
139+
throw InvalidSchemaException(message);
138140
}
139141

140142
begin_name = 2;
@@ -156,8 +158,9 @@ std::string DocumentNode::DoOneSubstitution(const std::string &text)
156158
c = text[begin_name];
157159
if (!std::isalpha(c) && c != '_')
158160
{
159-
OTEL_INTERNAL_LOG_ERROR(illegal_msg << ": " << text);
160-
throw InvalidSchemaException(illegal_msg);
161+
std::string message = illegal_msg;
162+
message.append(text);
163+
throw InvalidSchemaException(message);
161164
}
162165

163166
end_name = begin_name + 1;
@@ -190,8 +193,9 @@ std::string DocumentNode::DoOneSubstitution(const std::string &text)
190193
pos = text.find(replacement_token, end_name);
191194
if (pos != end_name)
192195
{
193-
OTEL_INTERNAL_LOG_ERROR(illegal_msg << ": " << text);
194-
throw InvalidSchemaException(illegal_msg);
196+
std::string message = illegal_msg;
197+
message.append(text);
198+
throw InvalidSchemaException(message);
195199
}
196200
// text is of the form ${ENV_NAME:-fallback}
197201
begin_fallback = pos + replacement_token.length();
@@ -215,7 +219,7 @@ std::string DocumentNode::DoOneSubstitution(const std::string &text)
215219
return fallback;
216220
}
217221

218-
bool DocumentNode::BooleanFromString(const std::string &value)
222+
bool DocumentNode::BooleanFromString(const std::string &value) const
219223
{
220224
if (value == "true")
221225
{
@@ -227,34 +231,37 @@ bool DocumentNode::BooleanFromString(const std::string &value)
227231
return false;
228232
}
229233

230-
OTEL_INTERNAL_LOG_ERROR("Illegal integer value: " << value);
231-
throw InvalidSchemaException("Illegal bool value");
234+
std::string message("Illegal boolean value: ");
235+
message.append(value);
236+
throw InvalidSchemaException(message);
232237
}
233238

234-
size_t DocumentNode::IntegerFromString(const std::string &value)
239+
size_t DocumentNode::IntegerFromString(const std::string &value) const
235240
{
236241
const char *ptr = value.c_str();
237242
char *end = nullptr;
238243
size_t len = value.length();
239244
size_t val = strtoll(ptr, &end, 10);
240245
if (ptr + len != end)
241246
{
242-
OTEL_INTERNAL_LOG_ERROR("Illegal integer value: " << value);
243-
throw InvalidSchemaException("Illegal integer value");
247+
std::string message("Illegal integer value: ");
248+
message.append(value);
249+
throw InvalidSchemaException(message);
244250
}
245251
return val;
246252
}
247253

248-
double DocumentNode::DoubleFromString(const std::string &value)
254+
double DocumentNode::DoubleFromString(const std::string &value) const
249255
{
250256
const char *ptr = value.c_str();
251257
char *end = nullptr;
252258
size_t len = value.length();
253259
double val = strtod(ptr, &end);
254260
if (ptr + len != end)
255261
{
256-
OTEL_INTERNAL_LOG_ERROR("Illegal double value: " << value);
257-
throw InvalidSchemaException("Illegal double value");
262+
std::string message("Illegal double value: ");
263+
message.append(value);
264+
throw InvalidSchemaException(message);
258265
}
259266
return val;
260267
}

‎sdk/src/configuration/ryml_document.cc‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,13 @@ std::unique_ptr<Document> RymlDocument::Parse(const std::string &source, const s
5353
return doc;
5454
}
5555

56-
RymlDocument *ryml_doc = new RymlDocument(tree);
57-
doc = std::unique_ptr<Document>(ryml_doc);
56+
doc = std::make_unique<RymlDocument>(tree);
5857
return doc;
5958
}
6059

6160
std::unique_ptr<DocumentNode> RymlDocument::GetRootNode()
6261
{
63-
RymlDocumentNode *ryml_node = new RymlDocumentNode(tree_.rootref(), 0);
64-
std::unique_ptr<DocumentNode> node(ryml_node);
62+
auto node = std::make_unique<RymlDocumentNode>(tree_.rootref(), 0);
6563
return node;
6664
}
6765

0 commit comments

Comments
 (0)