diff --git a/README.md b/README.md index afd496c..5ac7658 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,11 @@ crawlscope validate --url https://example.com --sitemap https://example.com/site Child sitemap indexes are supported automatically. +Set `CRAWLSCOPE_PROFILE_TOKEN` through the process environment or your secret +manager when the site protects diagnostic timing with `X-Profile-Token`. +Crawlscope intentionally has no `--profile-token` flag because command arguments +can be exposed through shell history and process listings. + Validation output is grouped for terminal scanning: ```text @@ -141,10 +146,69 @@ puts result.issues.to_a.map(&:message) - `urls`: sitemap URLs selected for validation - `pages`: fetched page snapshots - `issues`: structured issues with `code`, `severity`, `category`, `url`, and `message` +- `server_timing_summary`: aggregate response timing data when pages publish it `result.ok?` returns `false` when an error is present. Warnings and notices remain available through `result.issues` without making the result fail. +## Server Timing + +Crawlscope parses the +[`Server-Timing`](https://www.w3.org/TR/server-timing/) response header for both +HTTP and browser-rendered crawls. Each page exposes its parsed header through +`page.server_timing`: + +```ruby +page.server_timing.each do |metric| + puts [metric.name, metric.duration, metric.description].compact.join(": ") +end +``` + +The validation report adds a `Server Timing` section only when at least one page +publishes the header. It includes: + +- header coverage and the number of pages publishing durations +- sample and page counts with average, p50, p95, and maximum per metric +- non-duration signals, including cache status and routing descriptions +- the ten pages with the largest individual metric +- the number of malformed entries ignored during parsing + +`dur` values are reported as milliseconds, as recommended by the specification. +Crawlscope does not add durations together because metrics such as `total`, +`app`, and `db` may overlap. A page's worst-offender entry is its largest +individual metric instead. + +The current HTTP and browser transports expose response headers, not response +trailers. Metrics published only in a `Server-Timing` trailer are therefore not +available to Crawlscope. + +Rails 8 applications can enable `ActionDispatch::ServerTiming` with: + +```ruby +config.server_timing = true +``` + +Applications that expose timing only to authenticated diagnostics can set +`config.profile_token`. When this value is present, Crawlscope sends it as +`X-Profile-Token` on same-origin sitemap, HTTP, and browser document requests. +The header survives same-origin redirects but is removed before a cross-origin +redirect, and browser subresources never receive it. Keep the token in the host +application's secret store rather than a URL, command argument, report, or +checked-in configuration. + +`CRAWLSCOPE_PROFILE_TOKEN` is the portable default for Rails, the standalone +CLI, Rake tasks, and plain Ruby callers. Rails applications may instead assign +`config.profile_token` from encrypted credentials when that is their established +secret-management boundary; explicit configuration takes precedence over the +environment. + +New Rails applications enable it in development by default; production remains +opt-in. Rails publishes the Active Support notification names observed during +each request and sums repeated events with the same name. The exact metrics +therefore depend on the application and request, but commonly include controller, +view, database, and cache instrumentation. Crawlscope accepts Rails' dotted +metric names and reports each metric independently. + ## Rails Usage Run the install generator after adding the gem: @@ -162,10 +226,11 @@ Customize the `Crawlscope.configure` block inside the generated initializer: ```ruby Crawlscope.configure do |config| - config.base_url = -> { "https://example.com" } - config.sitemap_path = -> { Rails.public_path.join("sitemap.xml").to_s } - config.site_name = "Example" - config.schema_registry = -> { Crawlscope::SchemaRegistry.default } + config.base_url = -> { ENV.fetch("CRAWLSCOPE_BASE_URL", "http://localhost:3000") } + config.sitemap_path = lambda { + ENV.fetch("SITEMAP", "#{config.base_url.to_s.chomp("/")}/sitemap.xml") + } + config.site_name = ENV.fetch("CRAWLSCOPE_SITE_NAME", "Application") end ``` @@ -193,6 +258,7 @@ Available environment overrides: - `URL` - `SITEMAP` +- `CRAWLSCOPE_PROFILE_TOKEN` - `RULES=metadata,links` - `JS=1` or `RENDERER=browser` - `TIMEOUT=30` @@ -226,10 +292,10 @@ bundle exec rake 'crawlscope:validate:ldjson[https://example.com/article]' `crawlscope:validate` runs all default sitemap rules: indexability, metadata, structured data, uniqueness, content quality, and links. `URL` is the site -base. Without `SITEMAP`, Crawlscope uses the configured sitemap path, then -falls back to `/sitemap.xml`. With `SITEMAP`, Crawlscope uses `URL` as the site -base and validates URLs from that sitemap. `SITEMAP` may be a full URL or a -local file path. +base. Without `SITEMAP`, Crawlscope uses the configured sitemap URL, then fetches +`/sitemap.xml` from `URL` over HTTP. With `SITEMAP`, Crawlscope uses `URL` as +the site base and validates URLs from that sitemap. `SITEMAP` may be a full URL +or an explicitly selected local file path. Plain `rake` does not pass `--url` style flags to tasks. Use `URL=...` or the task-argument form above instead. diff --git a/UPGRADE.md b/UPGRADE.md index f3c9415..e718b28 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -6,6 +6,34 @@ behavior. ## Next Release +### Default sitemap is fetched over HTTP + +Crawlscope no longer prefers `public/sitemap.xml` when validating a localhost +application. Without an explicit `SITEMAP` or configured `sitemap_path`, it now +fetches `/sitemap.xml` from the configured base URL so the crawl observes the +live application and database state. + +The Rails installer now generates the same HTTP default. Regenerate or update +existing initializers that use `Rails.public_path.join("sitemap.xml")`. +Explicit local sitemap paths remain supported through `SITEMAP`, `--sitemap`, +or `config.sitemap_path`. + +`CRAWLSCOPE_PROFILE_TOKEN` is now the portable default for standalone CLI, Rake, +Rails, and plain Ruby usage. Explicit `config.profile_token` values still take +precedence. + +### Server-Timing report output + +No host configuration is required. When one or more responses publish a +`Server-Timing` header, the text report now includes an optional `Server Timing` +section. Host applications that parse report text should accept this additional +section. + +Parsed metrics are available through `page.server_timing`, and aggregate data is +available through `result.server_timing_summary`. Crawlscope interprets `dur` +values as milliseconds and ignores malformed entries while reporting their +count. + ### Ruby 3.3 is now required Crawlscope now depends on the current Async runtime for production async HTTP diff --git a/lib/crawlscope/browser.rb b/lib/crawlscope/browser.rb index 2e7db5a..7216570 100644 --- a/lib/crawlscope/browser.rb +++ b/lib/crawlscope/browser.rb @@ -4,13 +4,15 @@ module Crawlscope class Browser - def initialize(base_url:, timeout_seconds:, network_idle_timeout_seconds:, scroll_page:) + def initialize(base_url:, timeout_seconds:, network_idle_timeout_seconds:, scroll_page:, profile_token: nil) @base_url = base_url @timeout_seconds = timeout_seconds @network_idle_timeout_seconds = network_idle_timeout_seconds + @profile_token = profile_token @scroll_page = scroll_page @browser = build_browser @page = @browser.create_page + configure_profile_requests end def close @@ -72,6 +74,21 @@ def build_browser ) end + def configure_profile_requests + return if @profile_token.to_s.empty? + + @page.network.intercept(resource_type: :document) + @page.on(:request) do |request| + headers = RequestHeaders.add_profile_token( + request.headers.dup, + url: request.url, + base_url: @base_url, + profile_token: @profile_token + ) + request.continue(headers: headers.map { |name, value| {name: name.to_s, value: value.to_s} }) + end + end + def scroll_for_render @page.evaluate("(function() { if (document.body) { window.scrollTo(0, document.body.scrollHeight); } })()") wait_for_network_idle diff --git a/lib/crawlscope/configuration.rb b/lib/crawlscope/configuration.rb index 76df814..1ada579 100644 --- a/lib/crawlscope/configuration.rb +++ b/lib/crawlscope/configuration.rb @@ -11,7 +11,7 @@ class Configuration RENDERERS = %i[http browser].freeze DEFAULT_TIMEOUT_SECONDS = 20 - attr_writer :allowed_statuses, :base_url, :browser_factory, :concurrency, :fetch_executor, :network_idle_timeout_seconds, :output, :renderer, :rule_registry, :schema_registry, :scroll_page, :site_name, :sitemap_path, :timeout_seconds + attr_writer :allowed_statuses, :base_url, :browser_factory, :concurrency, :fetch_executor, :network_idle_timeout_seconds, :output, :profile_token, :renderer, :rule_registry, :schema_registry, :scroll_page, :site_name, :sitemap_path, :timeout_seconds def allowed_statuses value = resolve(@allowed_statuses) @@ -59,6 +59,10 @@ def output value.nil? ? $stdout : value end + def profile_token + resolve(@profile_token) || ENV["CRAWLSCOPE_PROFILE_TOKEN"] + end + def renderer value = resolve(@renderer) normalized_value = value.to_s.strip @@ -93,6 +97,7 @@ def audit(base_url: self.base_url, sitemap_path: self.sitemap_path, rule_names: concurrency: concurrency, fetch_executor: fetch_executor, network_idle_timeout_seconds: network_idle_timeout_seconds, + profile_token: profile_token, renderer: renderer, timeout_seconds: timeout_seconds, allowed_statuses: allowed_statuses, diff --git a/lib/crawlscope/crawl.rb b/lib/crawlscope/crawl.rb index 4bb42dc..bf2aea1 100644 --- a/lib/crawlscope/crawl.rb +++ b/lib/crawlscope/crawl.rb @@ -2,7 +2,7 @@ module Crawlscope class Crawl - def initialize(base_url:, sitemap_path:, rules:, schema_registry:, browser_factory: nil, concurrency: Configuration::DEFAULT_CONCURRENCY, fetch_executor: nil, network_idle_timeout_seconds: Configuration::DEFAULT_BROWSER_NETWORK_IDLE_TIMEOUT_SECONDS, renderer: :http, scroll_page: Configuration::DEFAULT_BROWSER_SCROLL_PAGE, timeout_seconds: Configuration::DEFAULT_TIMEOUT_SECONDS, allowed_statuses: Configuration::DEFAULT_ALLOWED_STATUSES) + def initialize(base_url:, sitemap_path:, rules:, schema_registry:, browser_factory: nil, concurrency: Configuration::DEFAULT_CONCURRENCY, fetch_executor: nil, network_idle_timeout_seconds: Configuration::DEFAULT_BROWSER_NETWORK_IDLE_TIMEOUT_SECONDS, profile_token: nil, renderer: :http, scroll_page: Configuration::DEFAULT_BROWSER_SCROLL_PAGE, timeout_seconds: Configuration::DEFAULT_TIMEOUT_SECONDS, allowed_statuses: Configuration::DEFAULT_ALLOWED_STATUSES) @base_url = base_url @sitemap_path = sitemap_path @rules = Array(rules) @@ -10,6 +10,7 @@ def initialize(base_url:, sitemap_path:, rules:, schema_registry:, browser_facto @browser_factory = browser_factory @concurrency = concurrency @network_idle_timeout_seconds = network_idle_timeout_seconds + @profile_token = profile_token @renderer = renderer.to_sym @fetch_executor = fetch_executor || default_fetch_executor @scroll_page = scroll_page @@ -49,6 +50,7 @@ def sitemap_urls adapter: http_adapter, concurrency: @concurrency, fetch_executor: @fetch_executor, + profile_token: @profile_token, timeout_seconds: @timeout_seconds ).urls(base_url: @base_url) raise ValidationError, "No URLs found in sitemap: #{@sitemap_path}" if urls.empty? @@ -61,6 +63,7 @@ def browser base_url: @base_url, timeout_seconds: @timeout_seconds, network_idle_timeout_seconds: @network_idle_timeout_seconds, + profile_token: @profile_token, scroll_page: @scroll_page ) rescue LoadError => error @@ -71,7 +74,7 @@ def page if @renderer == :browser (@browser_factory || method(:browser)).call else - Http.new(base_url: @base_url, timeout_seconds: @timeout_seconds, adapter: http_adapter) + Http.new(base_url: @base_url, timeout_seconds: @timeout_seconds, adapter: http_adapter, profile_token: @profile_token) end end diff --git a/lib/crawlscope/http.rb b/lib/crawlscope/http.rb index d9eaed7..745736d 100644 --- a/lib/crawlscope/http.rb +++ b/lib/crawlscope/http.rb @@ -10,10 +10,11 @@ class Http MAX_REDIRECTS = 5 USER_AGENT = "Mozilla/5.0 (compatible; Crawlscope/1.0)" - def initialize(base_url:, timeout_seconds:, adapter: nil) + def initialize(base_url:, timeout_seconds:, adapter: nil, profile_token: nil) @base_url = base_url @timeout_seconds = timeout_seconds @adapter = adapter + @profile_token = profile_token @connections_by_thread = Concurrent::Map.new end @@ -28,6 +29,12 @@ def close def fetch(url) response = connection.get(url) do |request| request.headers["User-Agent"] = USER_AGENT + RequestHeaders.add_profile_token( + request.headers, + url: url, + base_url: @base_url, + profile_token: @profile_token + ) end final_url = response.env.url.to_s @@ -67,7 +74,11 @@ def fetch(url) def connection @connections_by_thread.compute_if_absent(Thread.current.object_id) do Faraday.new do |faraday| - faraday.response :follow_redirects, limit: MAX_REDIRECTS + faraday.response( + :follow_redirects, + limit: MAX_REDIRECTS, + callback: RequestHeaders.method(:strip_profile_token_on_cross_origin_redirect) + ) faraday.options.timeout = @timeout_seconds faraday.options.open_timeout = @timeout_seconds faraday.adapter @adapter if @adapter diff --git a/lib/crawlscope/page.rb b/lib/crawlscope/page.rb index 1de0037..65ee75f 100644 --- a/lib/crawlscope/page.rb +++ b/lib/crawlscope/page.rb @@ -19,5 +19,13 @@ def initialize(url:, normalized_url:, final_url:, normalized_final_url:, status: def html? !doc.nil? end + + def header(name) + headers.find { |key, _value| key.to_s.casecmp?(name.to_s) }&.last + end + + def server_timing + @server_timing ||= ServerTiming.new(header("server-timing")) + end end end diff --git a/lib/crawlscope/reporter.rb b/lib/crawlscope/reporter.rb index 4c8e1c0..370fbf3 100644 --- a/lib/crawlscope/reporter.rb +++ b/lib/crawlscope/reporter.rb @@ -19,13 +19,18 @@ def report(result) if result.issues.size.zero? @io.puts("Status: OK") - return + else + @io.puts("Status: #{status_for(result.issues)}") + @io.puts("Issues: #{result.issues.size} total (#{severity_summary(result.issues)})") end - @io.puts("Status: #{status_for(result.issues)}") - @io.puts("Issues: #{result.issues.size} total (#{severity_summary(result.issues)})") - @io.puts("") + ServerTiming::Reporter.new(io: @io).report( + result.server_timing_summary, + base_url: result.base_url + ) + return if result.issues.size.zero? + @io.puts("") report_summary(result.issues) @io.puts("") report_issue_groups(result.issues, base_url: result.base_url) diff --git a/lib/crawlscope/request_headers.rb b/lib/crawlscope/request_headers.rb new file mode 100644 index 0000000..384b5c0 --- /dev/null +++ b/lib/crawlscope/request_headers.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "uri" + +module Crawlscope + module RequestHeaders + PROFILE_TOKEN = "X-Profile-Token" + + module_function + + def add_profile_token(headers, url:, base_url:, profile_token:) + return headers if profile_token.to_s.empty? + return headers unless same_origin?(url, base_url: base_url) + + headers[PROFILE_TOKEN] = profile_token + headers + end + + def strip_profile_token_on_cross_origin_redirect(response_env, request_env) + return if same_origin?(request_env[:url], base_url: response_env[:url]) + + request_env[:request_headers].delete(PROFILE_TOKEN) + end + + def same_origin?(url, base_url:) + url_uri = URI.join(base_url.to_s, url.to_s) + base_uri = URI.parse(base_url.to_s) + + [url_uri.scheme, url_uri.host, url_uri.port] == [base_uri.scheme, base_uri.host, base_uri.port] + rescue URI::Error + false + end + end +end diff --git a/lib/crawlscope/result.rb b/lib/crawlscope/result.rb index bf3f522..f2b09ba 100644 --- a/lib/crawlscope/result.rb +++ b/lib/crawlscope/result.rb @@ -5,5 +5,9 @@ module Crawlscope def ok? issues.none?(&:error?) end + + def server_timing_summary + ServerTiming::Summary.new(pages) + end end end diff --git a/lib/crawlscope/run.rb b/lib/crawlscope/run.rb index 6944c12..ed63522 100644 --- a/lib/crawlscope/run.rb +++ b/lib/crawlscope/run.rb @@ -44,17 +44,7 @@ def default_sitemap_path(base_url:) value = @configuration.sitemap_path return value unless value.to_s.strip.empty? - local_path = File.expand_path("public/sitemap.xml", Dir.pwd) - return local_path if local_path_default?(base_url: base_url) && File.exist?(local_path) - "#{base_url.to_s.chomp("/")}/sitemap.xml" end - - def local_path_default?(base_url:) - host = URI.parse(base_url.to_s).host.to_s - ["localhost", "127.0.0.1"].include?(host) - rescue URI::InvalidURIError - false - end end end diff --git a/lib/crawlscope/server_timing.rb b/lib/crawlscope/server_timing.rb new file mode 100644 index 0000000..96c26cb --- /dev/null +++ b/lib/crawlscope/server_timing.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true + +module Crawlscope + class ServerTiming + include Enumerable + + TOKEN = /\A[!#$%&'*+\-.^_`|~0-9A-Za-z]+\z/ + NUMBER = /\A-?(?:(?:\d+(?:\.\d*)?)|(?:\.\d+))(?:[eE][+-]?\d+)?\z/ + QUOTED_STRING = /\A"(?:[^"\\\x00-\x1F]|\\[\t\x20-\x7E])*"\z/ + EMPTY_LIST_MEMBER = /\A[ \t]*\z/ + + Metric = Data.define(:name, :duration, :description) do + def timed? + !duration.nil? + end + end + + attr_reader :invalid_count + + def initialize(header) + @header = header + @invalid_count = 0 + @metrics = parse.freeze + end + + def each(&block) + @metrics.each(&block) + end + + def empty? + @metrics.empty? + end + + def present? + !@header.nil? + end + + def size + @metrics.size + end + + private + + def parse + Array(@header).flatten.compact.flat_map do |header| + metrics_from(header.to_s) + end + end + + def metrics_from(header) + fields, complete = split(header, ",") + + unless complete + @invalid_count += 1 + fields.pop + end + + fields.filter_map do |field| + metric_from(field) unless EMPTY_LIST_MEMBER.match?(field) + end + end + + def metric_from(field) + parts, complete = split(field, ";") + name = parts.shift.to_s.strip + + if complete && TOKEN.match?(name) + metric(name, parameters_from(parts)) + else + @invalid_count += 1 + nil + end + end + + def parameters_from(parts) + parts.each_with_object({}) do |part, parameters| + name, value = part.strip.split("=", 2) + next unless name + + name = name.downcase + next if parameters.key?(name) + + parameters[name] = parameter_value(value.to_s.strip) + end + end + + def parameter_value(value) + if TOKEN.match?(value) + value + elsif QUOTED_STRING.match?(value) + value[1...-1].gsub(/\\(.)/m, '\1') + end + end + + def metric(name, parameters) + duration = duration_from(parameters["dur"]) + + if (parameters.key?("dur") && duration.nil?) || + (parameters.key?("desc") && parameters["desc"].nil?) + @invalid_count += 1 + nil + else + Metric.new( + name: name, + duration: duration, + description: parameters["desc"] + ) + end + end + + def duration_from(value) + if value && NUMBER.match?(value) + duration = Float(value) + duration if duration.finite? + end + end + + def split(value, delimiter) + fields, quoted, escaped = [+""], false, false + + value.each_char do |character| + if escaped + escaped = false + elsif quoted && character == "\\" + escaped = true + elsif character == "\"" + quoted = !quoted + elsif character == delimiter && !quoted + fields << +"" + end + + fields.last << character unless character == delimiter && !quoted + end + + [fields, !quoted && !escaped] + end + end +end diff --git a/lib/crawlscope/server_timing/reporter.rb b/lib/crawlscope/server_timing/reporter.rb new file mode 100644 index 0000000..405d0dc --- /dev/null +++ b/lib/crawlscope/server_timing/reporter.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +require "uri" + +module Crawlscope + class ServerTiming + class Reporter + def initialize(io:) + @io = io + end + + def report(summary, base_url:) + return unless summary.present? + + @io.puts("") + @io.puts("Server Timing:") + @io.puts( + " Coverage: #{summary.coverage}/#{summary.pages.size} pages " \ + "(#{percentage(summary.coverage, summary.pages.size)}); " \ + "durations on #{summary.duration_pages} #{pluralize(:page, summary.duration_pages)}" + ) + + report_metrics(summary.metrics) + report_signals(summary.signals) + report_worst_pages(summary.worst_pages, base_url: base_url) + @io.puts(" Ignored malformed entries: #{summary.invalid_count}") if summary.invalid_count.positive? + end + + private + + def report_metrics(metrics) + return if metrics.empty? + + @io.puts(" Duration metrics (dur; milliseconds assumed):") + metrics.each do |metric| + label = metric[:name] + label += " #{description(metric[:description])}" if metric[:description] + + @io.puts( + " #{label}: " \ + "#{metric[:samples]} #{pluralize(:sample, metric[:samples])} / " \ + "#{metric[:pages]} #{pluralize(:page, metric[:pages])}; " \ + "avg #{duration(metric[:average])}; " \ + "p50 #{duration(metric[:p50])}; " \ + "p95 #{duration(metric[:p95])}; " \ + "max #{duration(metric[:maximum])}" + ) + end + end + + def report_signals(signals) + return if signals.empty? + + @io.puts(" Signals:") + signals.each do |signal| + label = signal[:name] + label += " #{description(signal[:description])}" if signal[:description] + + @io.puts( + " #{label}: " \ + "#{signal[:samples]} #{pluralize(:sample, signal[:samples])} / " \ + "#{signal[:pages]} #{pluralize(:page, signal[:pages])}" + ) + end + end + + def report_worst_pages(samples, base_url:) + return if samples.empty? + + @io.puts(" Worst pages:") + samples.each do |sample| + metric = sample[:metric] + detail = metric.description ? " #{description(metric.description)}" : "" + @io.puts( + " #{relative_url(sample[:page].url, base_url: base_url)}: " \ + "#{duration(metric.duration)} (#{metric.name}#{detail})" + ) + end + end + + def duration(value) + "#{format("%.3f", value).sub(/\.?0+\z/, "")}ms" + end + + def description(value) + value = value.to_s + value = "#{value[0, 77]}..." if value.length > 80 + value.inspect + end + + def percentage(numerator, denominator) + return "0.0%" if denominator.zero? + + format("%.1f%%", numerator.fdiv(denominator) * 100) + end + + def relative_url(url, base_url:) + uri = URI.parse(url) + base_uri = URI.parse(base_url) + return url unless uri.host == base_uri.host && uri.scheme == base_uri.scheme && uri.port == base_uri.port + + uri.request_uri + rescue URI::InvalidURIError + url + end + + def pluralize(word, count) + (count == 1) ? word.to_s : "#{word}s" + end + end + end +end diff --git a/lib/crawlscope/server_timing/summary.rb b/lib/crawlscope/server_timing/summary.rb new file mode 100644 index 0000000..b5092ac --- /dev/null +++ b/lib/crawlscope/server_timing/summary.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +module Crawlscope + class ServerTiming + class Summary + WORST_PAGE_LIMIT = 10 + + attr_reader :pages + + def initialize(pages) + @pages = pages + @timings = pages.filter_map do |page| + timing = page.server_timing + [page, timing] if timing.present? + end + end + + def present? + @timings.any? + end + + def duration_pages + @timings.count { |_page, timing| timing.any?(&:timed?) } + end + + def invalid_count + @timings.sum { |_page, timing| timing.invalid_count } + end + + def metrics + timed_metrics + .group_by { |_page, metric| [metric.name, metric.description] } + .map { |(name, description), samples| metric_summary(name, description, samples) } + .sort_by { |metric| [-metric[:p95], -metric[:maximum], metric[:name]] } + end + + def signals + all_metrics + .reject { |_page, metric| metric.timed? } + .group_by { |_page, metric| [metric.name, metric.description] } + .map { |(name, description), samples| signal_summary(name, description, samples) } + .sort_by { |signal| [-signal[:samples], signal[:name], signal[:description].to_s] } + end + + def worst_pages(limit: WORST_PAGE_LIMIT) + @timings + .filter_map { |page, timing| worst_metric(page, timing) } + .min_by(limit) { |sample| [-sample[:metric].duration, sample[:page].url] } + .sort_by { |sample| [-sample[:metric].duration, sample[:page].url] } + end + + def coverage + @timings.size + end + + private + + def timed_metrics + all_metrics.select { |_page, metric| metric.timed? } + end + + def all_metrics + @all_metrics ||= @timings.flat_map do |page, timing| + timing.map { |metric| [page, metric] } + end + end + + def metric_summary(name, description, samples) + durations = samples.map { |_page, metric| metric.duration }.sort + + { + name: name, + description: description, + samples: samples.size, + pages: samples.map(&:first).uniq.size, + average: durations.sum.fdiv(durations.size), + p50: percentile(durations, 0.50), + p95: percentile(durations, 0.95), + maximum: durations.last + } + end + + def signal_summary(name, description, samples) + { + name: name, + description: description, + samples: samples.size, + pages: samples.map(&:first).uniq.size + } + end + + def percentile(values, percentile) + position = (values.size - 1) * percentile + lower = values[position.floor] + upper = values[position.ceil] + + lower + (upper - lower) * (position - position.floor) + end + + def worst_metric(page, timing) + if (metric = timing.select(&:timed?).max_by(&:duration)) + {page: page, metric: metric} + end + end + end + end +end diff --git a/lib/crawlscope/sitemap.rb b/lib/crawlscope/sitemap.rb index 8ebfb33..7eb7f69 100644 --- a/lib/crawlscope/sitemap.rb +++ b/lib/crawlscope/sitemap.rb @@ -9,11 +9,12 @@ module Crawlscope class Sitemap SITEMAP_NAMESPACE = {"xmlns" => "http://www.sitemaps.org/schemas/sitemap/0.9"}.freeze - def initialize(path:, adapter: nil, concurrency: Configuration::DEFAULT_CONCURRENCY, fetch_executor: Configuration::DEFAULT_FETCH_EXECUTOR, timeout_seconds: Configuration::DEFAULT_TIMEOUT_SECONDS) + def initialize(path:, adapter: nil, concurrency: Configuration::DEFAULT_CONCURRENCY, fetch_executor: Configuration::DEFAULT_FETCH_EXECUTOR, profile_token: nil, timeout_seconds: Configuration::DEFAULT_TIMEOUT_SECONDS) @path = path @adapter = adapter @concurrency = concurrency @fetch_executor = fetch_executor + @profile_token = profile_token @timeout_seconds = timeout_seconds end @@ -34,7 +35,7 @@ def collect_urls(source, base_url:, visited:, visited_mutex:) end return [] if already_visited - document = Nokogiri::XML(read(source)) + document = Nokogiri::XML(read(source, base_url: base_url)) root_name = document.root&.name unless %w[sitemapindex urlset].include?(root_name) raise ValidationError, "Sitemap #{source} has unexpected root #{root_name.inspect}" @@ -55,9 +56,16 @@ def collect_urls(source, base_url:, visited:, visited_mutex:) end end - def read(source) + def read(source, base_url:) if Url.remote?(source) - response = connection.get(source) + response = connection.get(source) do |request| + RequestHeaders.add_profile_token( + request.headers, + url: source, + base_url: base_url, + profile_token: @profile_token + ) + end unless response.status.to_i.between?(200, 299) raise ValidationError, "Sitemap #{source} returned HTTP #{response.status}" end @@ -92,7 +100,11 @@ def local_child_path(parent_source, child_loc) def connection Faraday.new do |faraday| - faraday.response :follow_redirects, limit: Http::MAX_REDIRECTS + faraday.response( + :follow_redirects, + limit: Http::MAX_REDIRECTS, + callback: RequestHeaders.method(:strip_profile_token_on_cross_origin_redirect) + ) faraday.options.timeout = @timeout_seconds faraday.options.open_timeout = @timeout_seconds faraday.adapter @adapter if @adapter diff --git a/lib/generators/crawlscope/templates/initializer.rb.tt b/lib/generators/crawlscope/templates/initializer.rb.tt index 1a18826..cd804ea 100644 --- a/lib/generators/crawlscope/templates/initializer.rb.tt +++ b/lib/generators/crawlscope/templates/initializer.rb.tt @@ -14,7 +14,7 @@ module CrawlscopeConfiguration Crawlscope.configure do |config| config.base_url = -> { ENV.fetch("CRAWLSCOPE_BASE_URL", "http://localhost:3000") } config.sitemap_path = lambda { - ENV.fetch("SITEMAP", Rails.public_path.join("sitemap.xml").to_s) + ENV.fetch("SITEMAP", "#{config.base_url.to_s.chomp("/")}/sitemap.xml") } config.site_name = ENV.fetch("CRAWLSCOPE_SITE_NAME", "Application") end diff --git a/test/crawlscope/browser_test.rb b/test/crawlscope/browser_test.rb index 4d6058a..48e1556 100644 --- a/test/crawlscope/browser_test.rb +++ b/test/crawlscope/browser_test.rb @@ -14,13 +14,14 @@ def quit end class FakeNetwork - attr_reader :cleared, :idle_waits, :status + attr_reader :cleared, :idle_waits, :interceptions, :status def initialize(response:, status: 200) @response = response @status = status @cleared = [] @idle_waits = [] + @interceptions = [] end def clear(scope) @@ -29,6 +30,10 @@ def clear(scope) attr_reader :response + def intercept(**options) + @interceptions << options + end + def wait_for_idle(duration:, timeout:) @idle_waits << {duration: duration, timeout: timeout} end @@ -43,6 +48,7 @@ def initialize(network:, body: "", current_url: "", url: "") @current_url = current_url @url = url @evaluations = [] + @callbacks = {} end attr_reader :body @@ -57,11 +63,40 @@ def go_to(url) @visited_url = url end + def on(event, &callback) + @callbacks[event] = callback + end + + def publish(event, value) + @callbacks.fetch(event).call(value) + end + attr_reader :url end + class FakeRequest + attr_reader :continued_headers, :headers, :url + + def initialize(url:, headers: {}) + @url = url + @headers = headers + end + + def continue(headers:) + @continued_headers = headers.to_h { |header| [header.fetch(:name), header.fetch(:value)] } + end + end + def test_fetch_returns_rendered_page - network = FakeNetwork.new(response: Response.new(url: "https://example.com/final", headers: {"content-type" => "text/html"})) + network = FakeNetwork.new( + response: Response.new( + url: "https://example.com/final", + headers: { + "content-type" => "text/html", + "server-timing" => "app;dur=47.2" + } + ) + ) page = FakePage.new(network: network, body: "
Hello") browser = browser_with(page: page, scroll_page: false) @@ -73,6 +108,7 @@ def test_fetch_returns_rendered_page assert_equal "https://example.com/final", result.normalized_final_url assert_equal 200, result.status assert result.html? + assert_equal 47.2, result.server_timing.first.duration assert_equal [], page.evaluations end @@ -88,6 +124,21 @@ def test_fetch_scrolls_when_enabled assert_equal 4, network.idle_waits.size end + def test_profile_token_is_limited_to_same_origin_documents + network = FakeNetwork.new(response: nil) + page = FakePage.new(network: network) + browser_with(page: page, profile_token: "profile-token") + same_origin_request = FakeRequest.new(url: "https://example.com/page") + cross_origin_request = FakeRequest.new(url: "https://cdn.example.net/frame") + + page.publish(:request, same_origin_request) + page.publish(:request, cross_origin_request) + + assert_equal [{resource_type: :document}], network.interceptions + assert_equal "profile-token", same_origin_request.continued_headers.fetch("X-Profile-Token") + refute_includes cross_origin_request.continued_headers, "X-Profile-Token" + end + def test_fetch_falls_back_to_page_url_and_original_url page_url_network = FakeNetwork.new(response: nil) page_url = FakePage.new(network: page_url_network, url: "https://example.com/page") @@ -142,14 +193,16 @@ def test_close_allows_missing_browser private - def browser_with(page: FakePage.new(network: FakeNetwork.new(response: nil)), browser: FakeBrowser.new, scroll_page: false) + def browser_with(page: FakePage.new(network: FakeNetwork.new(response: nil)), browser: FakeBrowser.new, profile_token: nil, scroll_page: false) Crawlscope::Browser.allocate.tap do |instance| instance.instance_variable_set(:@base_url, "https://example.com") instance.instance_variable_set(:@timeout_seconds, 20) instance.instance_variable_set(:@network_idle_timeout_seconds, 5) + instance.instance_variable_set(:@profile_token, profile_token) instance.instance_variable_set(:@scroll_page, scroll_page) instance.instance_variable_set(:@browser, browser) instance.instance_variable_set(:@page, page) + instance.send(:configure_profile_requests) end end end diff --git a/test/crawlscope/configuration_test.rb b/test/crawlscope/configuration_test.rb index 48d490c..a7ab6c9 100644 --- a/test/crawlscope/configuration_test.rb +++ b/test/crawlscope/configuration_test.rb @@ -14,6 +14,7 @@ def test_audit_builds_from_configured_callables config.site_name = -> { "Example" } config.concurrency = -> { 4 } config.fetch_executor = -> { :threaded } + config.profile_token = -> { "profile-token" } end audit = Crawlscope.configuration.audit @@ -22,6 +23,7 @@ def test_audit_builds_from_configured_callables assert_equal "/tmp/sitemap.xml", audit.instance_variable_get(:@sitemap_path) assert_equal 4, audit.instance_variable_get(:@concurrency) assert_equal :threaded, audit.instance_variable_get(:@fetch_executor) + assert_equal "profile-token", audit.instance_variable_get(:@profile_token) assert_equal %i[ indexability metadata @@ -53,17 +55,35 @@ def test_audit_raises_without_sitemap_path end def test_defaults_are_normalized - config = Crawlscope::Configuration.new + with_profile_token(nil) do + config = Crawlscope::Configuration.new + + assert_equal [200, 301, 302], config.allowed_statuses + assert_equal 10, config.concurrency + assert_equal :async, config.fetch_executor + assert_equal 4, config.browser_concurrency + assert_equal 5, config.network_idle_timeout_seconds + assert_equal :http, config.renderer + assert_equal 20, config.timeout_seconds + assert_equal $stdout, config.output + assert_nil config.profile_token + assert config.scroll_page? + end + end - assert_equal [200, 301, 302], config.allowed_statuses - assert_equal 10, config.concurrency - assert_equal :async, config.fetch_executor - assert_equal 4, config.browser_concurrency - assert_equal 5, config.network_idle_timeout_seconds - assert_equal :http, config.renderer - assert_equal 20, config.timeout_seconds - assert_equal $stdout, config.output - assert config.scroll_page? + def test_profile_token_defaults_to_the_environment + with_profile_token("environment-profile-token") do + assert_equal "environment-profile-token", Crawlscope::Configuration.new.profile_token + end + end + + def test_configured_profile_token_precedes_the_environment + with_profile_token("environment-profile-token") do + config = Crawlscope::Configuration.new + config.profile_token = "configured-profile-token" + + assert_equal "configured-profile-token", config.profile_token + end end def test_browser_renderer_defaults_to_threaded_fetch_executor @@ -119,4 +139,24 @@ def test_numeric_values_must_be_positive_integers assert_equal "Crawlscope concurrency must be an integer >= 1", error.message end + + private + + def with_profile_token(value) + previous_value = ENV["CRAWLSCOPE_PROFILE_TOKEN"] + + if value.nil? + ENV.delete("CRAWLSCOPE_PROFILE_TOKEN") + else + ENV["CRAWLSCOPE_PROFILE_TOKEN"] = value + end + + yield + ensure + if previous_value.nil? + ENV.delete("CRAWLSCOPE_PROFILE_TOKEN") + else + ENV["CRAWLSCOPE_PROFILE_TOKEN"] = previous_value + end + end end diff --git a/test/crawlscope/crawl_test.rb b/test/crawlscope/crawl_test.rb index 353cebd..842499a 100644 --- a/test/crawlscope/crawl_test.rb +++ b/test/crawlscope/crawl_test.rb @@ -167,6 +167,35 @@ def test_collects_metadata_issues_for_invalid_page ].sort, result.issues.to_a.map(&:code).uniq.sort end + def test_profile_token_reaches_remote_sitemap_and_page_requests + sitemap_request = stub_request(:get, "https://example.com/sitemap.xml") + .with(headers: {"X-Profile-Token" => "profile-token"}) + .to_return( + status: 200, + body: <<~XML + +