Skip to content

Commit 4e76d62

Browse files
hxiong388hxiong388
committed
Refactor Stackdriver::Core::Configuration to be less restrictive. Adopt this new configuration mechanism in Logging instrumentation
1 parent 0efbee9 commit 4e76d62

15 files changed

Lines changed: 217 additions & 206 deletions

File tree

‎google-cloud-error_reporting/lib/google/cloud/error_reporting.rb‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,19 +71,10 @@ module Cloud
7171
# for more examples.
7272
#
7373
module ErrorReporting
74-
##
75-
# @private Instrumentation library configuration options
76-
CONFIG_OPTIONS = %I{
77-
project_id
78-
keyfile
79-
service_name
80-
service_version
81-
}
82-
8374
# Initialize :error_reporting as a nested Configuration under
8475
# Google::Cloud if haven't already
8576
unless Google::Cloud.configure.option? :error_reporting
86-
Google::Cloud.configure.add_options error_reporting: CONFIG_OPTIONS
77+
Google::Cloud.configure.add_nested :error_reporting
8778
end
8879

8980
##

‎google-cloud-error_reporting/lib/google/cloud/error_reporting/middleware.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,8 @@ def error_event_from_exception env, exception
190190
# Sync Middleware configuration parameters from multiple sources. The
191191
# sources take precedence in following order: explicit parameters,
192192
# Google::Cloud::ErrorReporting.configure, Google::Cloud.configure,
193-
# then Google::Cloud::ErrorReporting::Project#defaults. This methods
194-
# sets final parameters as Middleware instance varaibles, then set
193+
# then Google::Cloud::ErrorReporting::Project#defaults. This method
194+
# sets final parameters as Middleware instance varaibles, then sets
195195
# the same parameters back into Google::Cloud::ErrorReporting, to ensure
196196
# the Middleware and the simple API configuration are consistent.
197197
#

‎google-cloud-error_reporting/test/google/cloud/error_reporting/middleware_test.rb‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,7 @@ class IgnoredError < ::StandardError
6767

6868
after {
6969
# Clear configuration values between each test
70-
config_hash = Google::Cloud::ErrorReporting.configure.instance_variable_get(:@configs)
71-
config_hash.each do |k, _|
72-
config_hash[k] = nil
73-
end
70+
Google::Cloud::ErrorReporting.configure.clear
7471
}
7572

7673
describe "#initialize" do

‎google-cloud-error_reporting/test/google/cloud/error_reporting_test.rb‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,13 @@
6767
end
6868

6969
describe ".configure" do
70-
it "initializes Google::Cloud.configure.error_reporting once" do
71-
error_reporting_config = Google::Cloud.configure.error_reporting
72-
Google::Cloud::ErrorReporting.configure
73-
Google::Cloud.configure.error_reporting.must_equal error_reporting_config
70+
it "has Google::Cloud.configure.error_reporting initialized already" do
71+
Google::Cloud.configure.option?(:error_reporting).must_equal true
7472
end
7573

7674
it "operates on the same Configuration object as Google::Cloud.configure.error_reporting" do
7775
Google::Cloud::ErrorReporting.configure.must_equal Google::Cloud.configure.error_reporting
7876
end
79-
80-
it "initialies ErrorReporting Configuration with all the valid operations" do
81-
Google::Cloud::ErrorReporting.configure.instance_variable_get(:@configs).keys.must_equal Google::Cloud::ErrorReporting::CONFIG_OPTIONS
82-
Google::Cloud.configure.error_reporting.instance_variable_get(:@configs).keys.must_equal Google::Cloud::ErrorReporting::CONFIG_OPTIONS
83-
end
8477
end
8578

8679
describe ".report" do

‎google-cloud-logging/lib/google/cloud/logging.rb‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
require "google-cloud-logging"
1717
require "google/cloud/logging/project"
18+
require "stackdriver/core"
1819

1920
module Google
2021
module Cloud
@@ -328,6 +329,14 @@ module Cloud
328329
# ```
329330
#
330331
module Logging
332+
# Initialize :error_reporting as a nested Configuration under
333+
# Google::Cloud if haven't already
334+
unless Google::Cloud.configure.option? :logging
335+
Google::Cloud.configure.add_nested logging: {
336+
monitored_resource: :labels
337+
}
338+
end
339+
331340
##
332341
# Creates a new object for connecting to the Stackdriver Logging service.
333342
# Each call creates a new connection.
@@ -379,6 +388,32 @@ def self.new project: nil, keyfile: nil, scope: nil, timeout: nil,
379388
project, credentials, timeout: timeout,
380389
client_config: client_config))
381390
end
391+
392+
##
393+
# Configure the Google::Cloud::Logging::Middleware when used in a
394+
# Rack-based application.
395+
#
396+
# Possible configuration parameters:
397+
# * `project_id`: The Google Cloud Project ID. Automatically discovered
398+
# when running from GCP environments.
399+
# * `keyfile`: The service account JSON file path. Automatically
400+
# discovered when running from GCP environments.
401+
# * `log_name`: A name for the log.
402+
# * `monitored_resource.type`: Identifier for monitored resource.
403+
# Optional. See [Monitored Resource
404+
# Doc](https://cloud.google.com/logging/docs/api/v2/resource-list)
405+
# for full list.
406+
# * `monitored_resource.labels`: Hash of custom labels for Monitored
407+
# Resource.
408+
#
409+
# @return [Stackdriver::Core::Configuration] The configuration object
410+
# the Google::Cloud::Logging module uses.
411+
#
412+
def self.configure
413+
yield Google::Cloud.configure.logging if block_given?
414+
415+
Google::Cloud.configure.logging
416+
end
382417
end
383418
end
384419
end

‎google-cloud-logging/lib/google/cloud/logging/middleware.rb‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ class Middleware
5050
# file path the file must be readable. Used to create a default logger
5151
# if one isn't specified and a service account is used for
5252
# authentication. Optional.
53+
# @param [String] log_name The name of the log to use. Use default
54+
# log name if nil.
55+
# @param [String] resource_type The name of Logging Monitored Resource.
56+
# Defaults to
5357
# @param [Hash] log_name_map A map from URI path to log name override.
5458
# The path may be a string or regex. If a request path matches an
5559
# entry in this map, the log name is set accordingly, otherwise the
@@ -59,17 +63,26 @@ class Middleware
5963
# Google::Cloud::Logging::Middleware instance
6064
#
6165
def initialize app, logger: nil, project_id: nil, keyfile: nil,
62-
log_name_map: DEFAULT_LOG_NAME_MAP
66+
log_name: nil, log_name_map: DEFAULT_LOG_NAME_MAP,
67+
resource_type: nil, resource_labels: nil
6368
@app = app
69+
@log_name_map = log_name_map
70+
71+
load_config project_id: project_id,
72+
keyfile: keyfile,
73+
log_name: log_name,
74+
resource_type: resource_type,
75+
resource_labels: resource_labels
76+
6477
if logger
6578
@logger = logger
6679
else
67-
logging = Google::Cloud::Logging.new project: project_id,
68-
keyfile: keyfile
69-
resource = Middleware.build_monitored_resource
70-
@logger = logging.logger DEFAULT_LOG_NAME, resource
80+
logging = Google::Cloud::Logging.new project: @project_id,
81+
keyfile: @keyfile
82+
resource = Middleware.build_monitored_resource @resource_type,
83+
@resource_labels
84+
@logger = logging.logger @log_name, resource
7185
end
72-
@log_name_map = log_name_map
7386
end
7487

7588
##
@@ -249,6 +262,34 @@ def self.default_monitored_resource
249262
end
250263

251264
private_class_method :default_monitored_resource
265+
266+
private
267+
268+
##
269+
# @private Sync Middleware configuration parameters from multiple
270+
# sources. The sources take precendence in the following order:
271+
# explicit parameters; Google::Cloud::Logging.configure;
272+
# Google::Cloud.configure; and finally the defaults.
273+
def load_config project_id: nil, keyfile: nil, log_name: nil,
274+
resource_type: nil, resource_labels: nil
275+
gcloud_config = Google::Cloud.configure
276+
logging_config = gcloud_config.logging
277+
278+
@project_id = project_id ||
279+
logging_config.project_id ||
280+
gcloud_config.project_id ||
281+
Logging::Project.default_project
282+
@keyfile = keyfile || logging_config.keyfile || gcloud_config.keyfile
283+
@log_name = log_name || logging_config.log_name || DEFAULT_LOG_NAME
284+
@resource_type = resource_type || logging_config.resource_type
285+
@resource_labels = resource_labels || logging_config.resource_labels
286+
287+
logging_config.project_id = @project_id
288+
logging_config.keyfile = @keyfile
289+
logging_config.log_name = @log_name
290+
logging_config.resource_type = @resource_type
291+
logging_config.resource_labels = @resource_labels
292+
end
252293
end
253294
end
254295
end

‎google-cloud-logging/lib/google/cloud/logging/rails.rb‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,9 @@ class Railtie < ::Rails::Railtie
6464
::ActiveSupport::OrderedOptions.new
6565

6666
initializer "Stackdriver.Logging", before: :initialize_logger do |app|
67-
if self.class.use_logging? app.config
68-
logging_config = Railtie.parse_rails_config config
67+
use_logging = self.class.use_logging? app.config
68+
if use_logging
69+
logging_config = Railtie.parse_rails_config app.config
6970

7071
project_id = logging_config[:project_id]
7172
keyfile = logging_config[:keyfile]
@@ -82,8 +83,15 @@ class Railtie < ::Rails::Railtie
8283
app.config.logger = logging.logger log_name, resource
8384
app.middleware.insert_before Rails::Rack::Logger,
8485
Google::Cloud::Logging::Middleware,
85-
logger: app.config.logger
86+
logger: app.config.logger,
87+
project_id: project_id,
88+
keyfile: keyfile,
89+
log_name: log_name,
90+
resource_type: resource_type,
91+
resource_labels: resource_labels
8692
end
93+
94+
Google::Cloud.configure.use_logging = use_logging
8795
end
8896

8997
##

‎google-cloud-logging/test/google/cloud/logging/rails_test.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
it "returns false if can't find non-empty project_id" do
3838
Google::Cloud::Logging::Credentials.stub :default, nil do
3939
Google::Cloud::Logging::Project.stub :default_project, nil do
40-
$stderr.stub :write, nil do
40+
STDOUT.stub :puts, nil do
4141
Google::Cloud::Logging::Railtie.use_logging?(@rails_config).must_equal false
4242
end
4343
end

‎google-cloud-logging/test/google/cloud/logging_test.rb‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,4 +166,14 @@
166166
end
167167
end
168168
end
169+
170+
describe ".configure" do
171+
it "has Google::Cloud.configure.logging initialized already" do
172+
Google::Cloud.configure.option?(:logging).must_equal true
173+
end
174+
175+
it "operates on the same Configuration object as Google::Cloud.configure.logging" do
176+
Google::Cloud::Logging.configure.must_equal Google::Cloud.configure.logging
177+
end
178+
end
169179
end

‎integration/rails4_app/Gemfile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ end
4747

4848
gem "google-cloud-bigquery", path: "../../google-cloud-bigquery"
4949
gem "google-cloud-core", path: "../../google-cloud-core"
50+
gem "google-cloud-debugger", path: "../../google-cloud-debugger"
5051
gem "google-cloud-env", path: "../../google-cloud-env"
5152
gem "google-cloud-datastore", path: "../../google-cloud-datastore"
5253
gem "google-cloud-debugger", path: "../../google-cloud-debugger"

0 commit comments

Comments
 (0)