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

Add a skeleton "http_profile" package #1036

Merged
merged 6 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 62 additions & 17 deletions .github/workflows/dart.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions pkgs/http_profile/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## 0.0.1

* Skeleton class and test definitions.
27 changes: 27 additions & 0 deletions pkgs/http_profile/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
Copyright 2023, the Dart project authors.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
met:

* Redistributions of source code must retain the above copyright
notice, this list of conditions and the following disclaimer.
* Redistributions in binary form must reproduce the above
copyright notice, this list of conditions and the following
disclaimer in the documentation and/or other materials provided
with the distribution.
* Neither the name of Google LLC nor the names of its
contributors may be used to endorse or promote products derived
from this software without specific prior written permission.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
2 changes: 2 additions & 0 deletions pkgs/http_profile/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
An **experimental** package that allows HTTP clients outside of the Dart SDK
to integrate with the DevTools Network tab.
33 changes: 33 additions & 0 deletions pkgs/http_profile/lib/http_profile.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:io';

/// Records information about an HTTP request.
brianquinlan marked this conversation as resolved.
Show resolved Hide resolved
final class HttpClientRequestProfile {
/// Determines whether HTTP profiling is enabled or not.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Avoid phrases for getters. For booleans we often start the doc with "Whether".

Suggested change
/// Determines whether HTTP profiling is enabled or not.
/// Whether HTTP profiling is enabled for the current zone's [HttpClient].

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we want to pull the zone concept in here - I think that this is actually per isolate. But I stared the sentence with "Whether".

///
/// The value can be changed programmatically or through the DevTools Network
/// UX.
static bool get profilingEnabled => HttpClient.enableTimelineLogging;
static set profilingEnabled(bool enabled) =>
HttpClient.enableTimelineLogging = enabled;

String? requestMethod;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you expand the commit message (I usually edit the first PR comment then copy it to the message when I squash/merge) to include more details about how this class will be used by the SDK/DevTools?

What code will fill these fields in?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the commit message.

String? requestUri;

HttpClientRequestProfile._();

/// If HTTP profiling is enabled, returns
/// a [HttpClientRequestProfile] otherwise returns `null`.
static HttpClientRequestProfile? profile() {
// Always return `null` in product mode so that the
// profiling code can be tree shaken away.
if (const bool.fromEnvironment('dart.vm.product') || !profilingEnabled) {
return null;
}
final requestProfile = HttpClientRequestProfile._();
return requestProfile;
}
}
14 changes: 14 additions & 0 deletions pkgs/http_profile/mono_pkg.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
sdk:
- pubspec
- dev

stages:
- analyze_and_format:
- analyze: --fatal-infos
- format:
sdk:
- dev
- unit_test:
- test: --platform vm
os:
- linux
15 changes: 15 additions & 0 deletions pkgs/http_profile/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: http_profile
description: >-
A library used by HTTP client authors to integrate with the DevTools
Network tab.
publish_to: none
repository: https://github.com/dart-lang/http/tree/master/pkgs/http_profile

environment:
sdk: '>=3.0.0 <4.0.0'
brianquinlan marked this conversation as resolved.
Show resolved Hide resolved

dependencies:
test: ^1.24.9

dev_dependencies:
dart_flutter_team_lints: ^2.1.1
22 changes: 22 additions & 0 deletions pkgs/http_profile/test/profiling_enabled_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:io';

import 'package:http_profile/http_profile.dart';
import 'package:test/test.dart';

void main() {
test('profiling enabled', () async {
HttpClientRequestProfile.profilingEnabled = true;
expect(HttpClient.enableTimelineLogging, true);
expect(HttpClientRequestProfile.profile(), isNotNull);
});

test('profiling disabled', () async {
HttpClientRequestProfile.profilingEnabled = false;
expect(HttpClient.enableTimelineLogging, false);
expect(HttpClientRequestProfile.profile(), isNull);
});
}
2 changes: 1 addition & 1 deletion tool/ci.sh

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.