Skip to content

Commit a376b2c

Browse files
nebhalenebhale
committed
Polishing
This change polishes up the contribution to meet our coding standards. [resolves #436]
1 parent 7fa6806 commit a376b2c

6 files changed

Lines changed: 58 additions & 64 deletions

File tree

‎docs/framework-java_memory_assistant.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ Tags are printed to standard output by the buildpack detect script.
1515
## Configuration
1616
For general information on configuring the buildpack, including how to specify configuration values through environment variables, refer to [Configuration and Extension][].
1717

18-
The framework can be configured by modifying the [``config/java_memory_assistant.yml``][] file in the buildpack fork.
18+
The framework can be configured by modifying the [`config/java_memory_assistant.yml`][] file in the buildpack fork.
1919

2020
| Name | Description
2121
| ---- | -----------
@@ -47,7 +47,7 @@ The timestamp pattern `%ts:yyyyMMdd'T'mmssSSSZ%` is equivalent to the `%FT%T%z`
4747
| Survivor | `survivor` |
4848
| Old Generation | `old_gen` |
4949

50-
The default values can be found in the [``config/java_memory_assistant.yml``][] file.
50+
The default values can be found in the [`config/java_memory_assistant.yml`][] file.
5151

5252
### Examples
5353

@@ -112,4 +112,6 @@ To prevent heap dumps from "going down" with the container, you should consider
112112

113113
#### Container-mounted volumes
114114

115-
If you are using a filesystem service that mounts persistent volumes to the container, it is enough to name one of the volume services `heap-dump` or tag one volume with `heap-dump`, and the path specified as the `heap_dump_folder` configuration will be resolved against `<mount-point>/<space_name>-<space_id[0,8]>/<application_name>-<application_id[0-8]>`.
115+
If you are using a filesystem service that mounts persistent volumes to the container, it is enough to name one of the volume services `heap-dump` or tag one volume with `heap-dump`, and the path specified as the `heap_dump_folder` configuration will be resolved against `<mount-point>/<space_name>-<space_id[0,8]>/<application_name>-<application_id[0-8]>`.
116+
117+
[`config/java_memory_assistant.yml`]: ../config/java_memory_assistant.yml

‎lib/java_buildpack/framework/java_memory_assistant.rb‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@ class JavaMemoryAssistant < JavaBuildpack::Component::ModularComponent
2828
protected
2929

3030
# (see JavaBuildpack::Component::ModularComponent#command)
31-
def command
32-
# Nothing to do here, the agent is initialized via java opts
33-
end
31+
def command; end
3432

3533
# (see JavaBuildpack::Component::ModularComponent#sub_components)
3634
def sub_components(context)

‎lib/java_buildpack/framework/java_memory_assistant/agent.rb‎

Lines changed: 32 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -34,67 +34,64 @@ def compile
3434

3535
# (see JavaBuildpack::Component::BaseComponent#release)
3636
def release
37-
@droplet.java_opts.add_javaagent @droplet.sandbox + jar_name
38-
39-
@droplet.java_opts.add_system_property 'jma.enabled', 'true'
40-
@droplet.java_opts.add_system_property 'jma.heap_dump_name', %("#{name_pattern}")
37+
@droplet.java_opts
38+
.add_javaagent(@droplet.sandbox + jar_name)
39+
.add_system_property('jma.enabled', 'true')
40+
.add_system_property('jma.heap_dump_name', %("#{name_pattern}"))
41+
.add_system_property 'jma.log_level', normalized_log_level
4142

4243
add_system_prop_if_config_present 'check_interval', 'jma.check_interval'
4344
add_system_prop_if_config_present 'max_frequency', 'jma.max_frequency'
4445

45-
@droplet.java_opts.add_system_property 'jma.log_level', log_level
46+
return unless @configuration.key?('thresholds')
4647

47-
(@configuration['thresholds'] || {}).each do |key, value|
48+
@configuration['thresholds'].each do |key, value|
4849
@droplet.java_opts.add_system_property "jma.thresholds.#{key}", value.to_s
4950
end
5051
end
5152

5253
protected
5354

54-
def supports?
55-
true
56-
end
57-
5855
# (see JavaBuildpack::Component::VersionedDependencyComponent#jar_name)
5956
def jar_name
6057
"java-memory-assistant-#{@version}.jar"
6158
end
6259

63-
private
64-
65-
def name_pattern
66-
# Double escaping quotes of doom. Nothing less would work.
67-
%q(%env:CF_INSTANCE_INDEX%-%ts:yyyy-MM-dd'"'"'T'"'"'mm'"'"':'"'"'ss'"'"':'"'"'SSSZ%-) \
68-
'%env:CF_INSTANCE_GUID[,8]%.hprof'
60+
def supports?
61+
true
6962
end
7063

71-
def add_system_prop_if_config_present(config_entry, system_property_name, quote_value = false)
72-
return unless @configuration[config_entry]
64+
private
65+
66+
LOG_LEVEL_MAPPING = {
67+
'DEBUG' => 'DEBUG',
68+
'WARN' => 'WARNING',
69+
'INFO' => 'INFO',
70+
'ERROR' => 'ERROR',
71+
'FATAL' => 'ERROR'
72+
}.freeze
7373

74-
config_value = @configuration[config_entry]
75-
config_value = '"' + config_value + '"' if quote_value
74+
private_constant :LOG_LEVEL_MAPPING
7675

77-
@droplet.java_opts.add_system_property(system_property_name, config_value)
76+
def add_system_prop_if_config_present(config_entry, system_property_name)
77+
return unless @configuration.key?(config_entry)
78+
@droplet.java_opts.add_system_property(system_property_name, @configuration[config_entry])
7879
end
7980

8081
def log_level
81-
actual_log_level = @configuration['log_level'] || ENV['JBP_LOG_LEVEL'] || 'ERROR'
82-
83-
mapped_log_level = log_level_mapping[actual_log_level.upcase]
84-
85-
raise "Invalid value of the 'log_level' property: '#{actual_log_level}'" unless mapped_log_level
82+
@configuration['log_level'] || ENV['JBP_LOG_LEVEL'] || 'ERROR'
83+
end
8684

87-
mapped_log_level
85+
def normalized_log_level
86+
normalized_log_level = LOG_LEVEL_MAPPING[log_level.upcase]
87+
raise "Invalid value of the 'log_level' property: '#{log_level}'" unless normalized_log_level
88+
normalized_log_level
8889
end
8990

90-
def log_level_mapping
91-
{
92-
'DEBUG' => 'DEBUG',
93-
'WARN' => 'WARNING',
94-
'INFO' => 'INFO',
95-
'ERROR' => 'ERROR',
96-
'FATAL' => 'ERROR'
97-
}
91+
def name_pattern
92+
# Double escaping quotes of doom. Nothing less would work.
93+
%q(%env:CF_INSTANCE_INDEX%-%ts:yyyy-MM-dd'"'"'T'"'"'mm'"'"':'"'"'ss'"'"':'"'"'SSSZ%-) \
94+
'%env:CF_INSTANCE_GUID[,8]%.hprof'
9895
end
9996

10097
end

‎lib/java_buildpack/framework/java_memory_assistant/clean_up.rb‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,18 @@ def compile
2828
return unless supports?
2929

3030
download_zip false
31-
32-
@droplet.copy_resources
3331
end
3432

3533
# (see JavaBuildpack::Component::BaseComponent#release)
3634
def release
3735
return unless supports?
3836

39-
environment_variables = @droplet.environment_variables
40-
environment_variables.add_environment_variable 'JMA_MAX_DUMP_COUNT', @configuration['max_dump_count'].to_s
37+
@droplet.environment_variables
38+
.add_environment_variable 'JMA_MAX_DUMP_COUNT', @configuration['max_dump_count'].to_s
4139

42-
@droplet.java_opts.add_system_property('jma.command.interpreter', '')
43-
@droplet.java_opts.add_system_property('jma.execute.before', @droplet.sandbox + 'cleanup')
40+
@droplet.java_opts
41+
.add_system_property('jma.command.interpreter', '')
42+
.add_system_property('jma.execute.before', @droplet.sandbox + 'cleanup')
4443
end
4544

4645
protected

‎lib/java_buildpack/framework/java_memory_assistant/heap_dump_folder.rb‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,7 @@ def detect
3838
end
3939

4040
# (see JavaBuildpack::Component::BaseComponent#compile)
41-
def compile
42-
# Nothing to do
43-
end
41+
def compile; end
4442

4543
# (see JavaBuildpack::Component::BaseComponent#release)
4644
def release

‎spec/java_buildpack/framework/java_memory_assistant/heap_dump_folder_spec.rb‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,13 @@
8282

8383
before do
8484
allow(services).to receive(:find_service).with('heap-dump')
85-
.and_return('volume_mounts' =>
86-
[
87-
{
88-
'container_dir' => '/my_volume',
89-
'mode' => 'rw'
90-
}
91-
])
85+
.and_return('volume_mounts' =>
86+
[
87+
{
88+
'container_dir' => '/my_volume',
89+
'mode' => 'rw'
90+
}
91+
])
9292
end
9393

9494
it 'adds \'jma.heap_dump_folder\' with volume container_dir as path root', :enable_log_file, log_level: 'INFO' do
@@ -112,13 +112,13 @@
112112

113113
before do
114114
allow(services).to receive(:find_service).with('heap-dump')
115-
.and_return('volume_mounts' =>
116-
[
117-
{
118-
'container_dir' => '/my_volume',
119-
'mode' => 'r'
120-
}
121-
])
115+
.and_return('volume_mounts' =>
116+
[
117+
{
118+
'container_dir' => '/my_volume',
119+
'mode' => 'r'
120+
}
121+
])
122122
end
123123

124124
it 'fails if volume does not have write mode active', :enable_log_file, log_level: 'DEBUG' do

0 commit comments

Comments
 (0)